Skip to content

fixed numpy issue#2078

Merged
niveditasing merged 7 commits into
datacommonsorg:masterfrom
niveditasing:test_faliure_fix
Jun 29, 2026
Merged

fixed numpy issue#2078
niveditasing merged 7 commits into
datacommonsorg:masterfrom
niveditasing:test_faliure_fix

Conversation

@niveditasing

@niveditasing niveditasing commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

A recent update in NumPy (version 2.0+) broke how it handles 2D math operations. This broke an external library we use called rdp (which simplifies geographical boundaries), causing our automated tests to crash with a ValueError.

FIX
Pinned NumPy to <=2.0.0 in requirements.txt to maintain compatibility with the rdp library.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a monkey-patch for np.cross in simplify.py to maintain compatibility with NumPy 2.x. However, the monkey-patch contains a correctness bug where using len() on a 2D NumPy array returns the size of the first dimension rather than the vector dimension, causing it to fall back to the original function and raise a ValueError. A more robust implementation checking the array shape is recommended.

Comment thread scripts/us_census/geojsons_low_res/simplify.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a monkey-patch for np.cross in simplify.py to maintain compatibility with NumPy 2.x, which removed support for 2D vectors. The reviewer suggested refining the monkey-patch to only apply the custom 2D cross-product logic when no extra positional or keyword arguments are provided, preventing potential silent bugs in other parts of the codebase that might call np.cross with custom axis arguments.

Comment thread scripts/us_census/geojsons_low_res/simplify.py Outdated
@balit-raibot

Copy link
Copy Markdown
Contributor

@gemini-code-assist review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a monkey-patch for np.cross in simplify.py to support 2D vectors under NumPy 2.x. The reviewer noted that applying the monkey-patch after importing rdp could fail if rdp imports cross directly from numpy during its own initialization. To resolve this, the reviewer suggested moving the monkey-patching logic to run immediately after importing numpy and before importing other third-party libraries.

Comment thread scripts/us_census/geojsons_low_res/simplify.py Outdated
Comment thread import-automation/executor/requirements.txt Outdated
@pulkeet-wq

Copy link
Copy Markdown
Contributor

LGTM

@niveditasing niveditasing merged commit 29533fb into datacommonsorg:master Jun 29, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants