You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Alex-PLACET (via GitHub)" <gi...@apache.org> on 2025/07/09 08:29:47 UTC
[PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Alex-PLACET opened a new pull request, #47034:
URL: https://github.com/apache/arrow/pull/47034
### Rationale for this change
Fix incorrect array generation in Archery
### What changes are included in this PR?
Fix an issue issue in Archery Integration which generate wrong number of values for run-end encoded.
### Are these changes tested?
In the CI
### Are there any user-facing changes?
The JSONs generated by Archery integration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #47034:
URL: https://github.com/apache/arrow/pull/47034#discussion_r2298271773
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -1157,7 +1164,7 @@ def _get_children(self):
]
def generate_column(self, size, name=None):
- values = self.values_field.generate_column(size)
+ values = self.values_field.generate_column(int(size/2))
run_ends = self.run_ends_field.generate_column(size)
if name is None:
name = self.name
Review Comment:
Ok, I'm now convinced that this is incorrect.
I think we should instead have something like:
```python
def generate_column(self, size, name=None):
num_run_ends = size // 4
values = self.values_field.generate_column(num_run_ends)
run_ends = self.run_ends_field.generate_column(num_run_ends, size)
if name is None:
name = self.name
return RunEndEncodedColumn(name, size, run_ends, values)
```
and in `RunEndsField`:
```python
def generate_column(self, size, logical_length, name=None):
assert logical_length < 2**(self.bit_width - 1)
rng = np.random.default_rng()
# Generate values that are strictly increasing with a min-value of 1.
# We sort the values to ensure they are strictly increasing and set
# replace to False to avoid duplicates, ensuring a valid run-ends array.
values = rng.choice(logical_length - 1, size=size, replace=False)
values += 1
values = sorted(values)
values = list(map(int if self.bit_width < 64 else str, values))
# RunEnds cannot be null, as such self.nullable == False and this
# will generate a validity map of all ones.
is_valid = self._make_is_valid(size)
if name is None:
name = self.name
return PrimitiveColumn(name, size, is_valid, values)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "Alex-PLACET (via GitHub)" <gi...@apache.org>.
Alex-PLACET commented on PR #47034:
URL: https://github.com/apache/arrow/pull/47034#issuecomment-3334610892
@pitrou Looks good to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #47034:
URL: https://github.com/apache/arrow/pull/47034#discussion_r2298274503
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -1157,7 +1164,7 @@ def _get_children(self):
]
def generate_column(self, size, name=None):
- values = self.values_field.generate_column(size)
+ values = self.values_field.generate_column(int(size/2))
run_ends = self.run_ends_field.generate_column(size)
if name is None:
name = self.name
Review Comment:
@paleolimbot @zeroshade What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47034:
URL: https://github.com/apache/arrow/pull/47034#issuecomment-3051677286
:warning: GitHub issue #47029 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47034:
URL: https://github.com/apache/arrow/pull/47034#issuecomment-3334355793
I've opened an alternate PR at https://github.com/apache/arrow/pull/47653, would you like to take a look @Alex-PLACET ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou closed pull request #47034: GH-47029: [Archery] Fix generation of RunEndsField
URL: https://github.com/apache/arrow/pull/47034
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #47034:
URL: https://github.com/apache/arrow/pull/47034#discussion_r2197171556
##########
dev/archery/archery/integration/datagen.py:
##########
@@ -202,25 +202,32 @@ def __init__(self, name, bit_width, *, metadata=None):
super().__init__(name, is_signed=True, bit_width=bit_width,
nullable=False, metadata=metadata, min_value=1)
- def generate_range(self, size, lower, upper, name=None,
- include_extremes=False):
+ def generate_range(self, size, lower, upper, name=None, include_extremes=False):
+ if size > (1 << self.bit_width) - 1:
+ raise ValueError(
+ f"Size {size} exceeds the maximum for bit width {self.bit_width}."
+ )
rng = np.random.default_rng()
- # generate values that are strictly increasing with a min-value of
- # 1, but don't go higher than the max signed value for the given
- # bit width. We sort the values to ensure they are strictly increasing
- # and set replace to False to avoid duplicates, ensuring a valid
- # run-ends array.
- values = rng.choice(2 ** (self.bit_width - 1) - 1, size=size, replace=False)
- values += 1
- values = sorted(values)
+ if size == 0:
+ values = []
+ runs_count = 0
+ elif size == 1:
+ values = [size]
+ runs_count = 1
+ else:
+ runs_count = int(size / 2)
+ values = rng.choice(range(1, size), size=runs_count - 1, replace=False)
+ values = sorted(values)
+ values.append(size)
+
values = list(map(int if self.bit_width < 64 else str, values))
# RunEnds cannot be null, as such self.nullable == False and this
# will generate a validity map of all ones.
- is_valid = self._make_is_valid(size)
+ is_valid = self._make_is_valid(runs_count)
if name is None:
name = self.name
- return PrimitiveColumn(name, size, is_valid, values)
+ return PrimitiveColumn(name, int(runs_count), is_valid, values)
Review Comment:
This seems ok on the face of it, but needs an adaptation on the C++ side if we want the decoded REE array to have a `length` equal to `runs_count`.
The alternative would be pass to `runs_count` here, which might also work better for Go and nanoarrow (the two other Arrow implementations that support REE currently).
@paleolimbot @zeroshade What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47029: [Archery] Fix generation of RunEndsField [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #47034:
URL: https://github.com/apache/arrow/pull/47034#issuecomment-4250984147
This PR was obsoleted by https://github.com/apache/arrow/pull/47653, closing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org