Skip to content
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

Change _GeoSeriesUtility._from_data(index=) default to None #1400

Merged
merged 6 commits into from
Jul 8, 2024

Conversation

mroeschke
Copy link
Contributor

@mroeschke mroeschke commented Jul 2, 2024

Description

An upstream change in cudf uncovered a bug in _GeoSeriesUtility._from_data where False was being passed to cudf.Series(index=) which is an invalid value. The only valid index values are an actual cudf.Index or None, so setting to None as a more appropriate default value

Also updates the location of assert_eq which moved in rapidsai/cudf#16063

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mroeschke mroeschke requested a review from a team as a code owner July 2, 2024 20:21
@mroeschke mroeschke requested review from trxcllnt and isVoid July 2, 2024 20:21
@github-actions github-actions bot added the Python Related to Python code label Jul 2, 2024
@vyasr vyasr added bug Something isn't working non-breaking Non-breaking change labels Jul 2, 2024
@harrism
Copy link
Member

harrism commented Jul 2, 2024

/merge

@mroeschke
Copy link
Contributor Author

There is one more (potential) cudf bug that I am diagnosing to get the CI to green.

@mroeschke
Copy link
Contributor Author

rapidsai/cudf#16193 should unblock the remaining failure here

rapids-bot bot pushed a commit to rapidsai/cudf that referenced this pull request Jul 4, 2024
The offset column of a nested empty list column may be empty as discussed in #16164. `ListColumn.memory_usage` assumed that this column was non-empty

Unblocks rapidsai/cuspatial#1400

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Lawrence Mitchell (https://github.com/wence-)

URL: #16193
@bdice
Copy link
Contributor

bdice commented Jul 8, 2024

@mroeschke There are a few more CI failures here like:

=========================== short test summary info ============================
FAILED tests/test_geoseries.py::test_memory_usage_large - assert 216789 == 216793
 +  where 216789 = <bound method Series.memory_usage of 0      MULTIPOLYGON (((180 -16.06713, 180 -16.55522, ...\n1      POLYGON ((33.9037...0.89, -60.895...\n176    POLYGON ((30.83385 3.50917, 29.9535 4.1737, 29...\nName: geometry, Length: 177, dtype: geometry>()
 +    where <bound method Series.memory_usage of 0      MULTIPOLYGON (((180 -16.06713, 180 -16.55522, ...\n1      POLYGON ((33.9037...0.89, -60.895...\n176    POLYGON ((30.83385 3.50917, 29.9535 4.1737, 29...\nName: geometry, Length: 177, dtype: geometry> = 0      MULTIPOLYGON (((180 -16.06713, 180 -16.55522, ...\n1      POLYGON ((33.90371 -0.95, 34.07262 -1.05982, 3...\n2   ...10.89, -60.895...\n176    POLYGON ((30.83385 3.50917, 29.9535 4.1737, 29...\nName: geometry, Length: 177, dtype: geometry.memory_usage
= 1 failed, 1574 passed, 19 skipped, 4 xfailed, 933 warnings in 187.45s (0:03:07) =

Can you investigate? I think this PR might be blocking #1402.

@bdice bdice mentioned this pull request Jul 8, 2024
3 tasks
@jameslamb jameslamb mentioned this pull request Jul 8, 2024
@mroeschke mroeschke requested a review from a team as a code owner July 8, 2024 21:02
@mroeschke mroeschke requested a review from AyodeAwe July 8, 2024 21:02
Comment on lines +338 to +343
# TODO: Remove geopandas.dataset usage in cuspatial_api_examples.ipynb
test_notebooks:
common:
- output_types: [conda, requirements, pyproject]
packages:
- geopandas<1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mroeschke Can you open an issue documenting the geopandas<1 pinning and whatever work items need to be done there?

I am going to merge this once CI passes to unblock #1402 since it is blocking other RAPIDS-CCCL work, just want to be sure that this is visible and has a corresponding issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing opened #1403

@bdice
Copy link
Contributor

bdice commented Jul 8, 2024

/merge

@rapids-bot rapids-bot bot merged commit e51e8b0 into rapidsai:branch-24.08 Jul 8, 2024
70 checks passed
@mroeschke mroeschke deleted the fix/_from_data branch July 8, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change Python Related to Python code
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

5 participants