-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[Data] Improve handling of pandas.NA
#53859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances how pandas.NA
values are handled by converting them to Python None
when rows are materialized as dictionaries.
- Adds a new test verifying
pd.NA
→None
conversion initer_rows
. - Makes
TableRow.as_pydict
an abstract method. - Implements
as_pydict
in the pandas block accessor to translatepd.NA
toNone
.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
python/ray/data/tests/test_pandas_block.py | Added test_iter_rows_with_na to cover pd.NA → None conversion |
python/ray/data/_internal/row.py | Marked as_pydict as an abstract method on TableRow |
python/ray/data/_internal/pandas_block.py | Implemented as_pydict to map pd.NA values to None |
Comments suppressed due to low confidence (3)
python/ray/data/tests/test_pandas_block.py:465
- The comment mentions 'NaN values' but this test is for
pd.NA
. Please update it to 'NA values' for clarity.
# We should return None for NaN values.
python/ray/data/_internal/pandas_block.py:113
- Type annotations
Dict
andAny
are used here but not imported in this module; addfrom typing import Any, Dict
at the top to prevent aNameError
.
def as_pydict(self) -> Dict[str, Any]:
python/ray/data/_internal/row.py:34
- [nitpick] Using an
Ellipsis
stub in an abstract method can mask missing implementations. Consider raisingNotImplementedError
or usingpass
to make the intent explicit.
...
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
<!-- Thank you for your contribution! Please review https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before opening a pull request. --> <!-- Please add a reviewer to the assignee section when you create a PR. If you don't have the access to it, we will shortly find a reviewer and assign them to your PR. --> ## Why are these changes needed? <!-- Please give a short summary of the change and the problem this solves. --> This change standardizes null handling across internal block formats by converting pd.NA to None. Since different formats (e.g., Pandas and Arrow) use different null representations, this ensures consistent semantics when converting between them. In particular, we treat pd.NA as a null value, while preserving np.nan as a distinct floating-point NaN. ## Related issue number <!-- For example: "Closes #1234" --> ## Checks - [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [ ] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
<!-- Thank you for your contribution! Please review https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before opening a pull request. --> <!-- Please add a reviewer to the assignee section when you create a PR. If you don't have the access to it, we will shortly find a reviewer and assign them to your PR. --> ## Why are these changes needed? <!-- Please give a short summary of the change and the problem this solves. --> This change standardizes null handling across internal block formats by converting pd.NA to None. Since different formats (e.g., Pandas and Arrow) use different null representations, this ensures consistent semantics when converting between them. In particular, we treat pd.NA as a null value, while preserving np.nan as a distinct floating-point NaN. ## Related issue number <!-- For example: "Closes ray-project#1234" --> ## Checks - [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [ ] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
<!-- Thank you for your contribution! Please review https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before opening a pull request. --> <!-- Please add a reviewer to the assignee section when you create a PR. If you don't have the access to it, we will shortly find a reviewer and assign them to your PR. --> ## Why are these changes needed? <!-- Please give a short summary of the change and the problem this solves. --> This change standardizes null handling across internal block formats by converting pd.NA to None. Since different formats (e.g., Pandas and Arrow) use different null representations, this ensures consistent semantics when converting between them. In particular, we treat pd.NA as a null value, while preserving np.nan as a distinct floating-point NaN. ## Related issue number <!-- For example: "Closes #1234" --> ## Checks - [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [ ] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Why are these changes needed?
This change standardizes null handling across internal block formats by converting pd.NA to None. Since different formats (e.g., Pandas and Arrow) use different null representations, this ensures consistent semantics when converting between them. In particular, we treat pd.NA as a null value, while preserving np.nan as a distinct floating-point NaN.
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.