-
Notifications
You must be signed in to change notification settings - Fork 117
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
SNOW-1487311: Fix test failures and lint issues with numpy 2 #1791
Conversation
@@ -92,7 +92,6 @@ | |||
(np.half, FloatType()), | |||
(np.float16, FloatType()), | |||
(np.float64, DoubleType()), | |||
(np.float_, DoubleType()), |
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.
oh, we actually removed np.float_, i thought he message says it is replaced with np.float?
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.
np.float_
is an alias for np.float64
after numpy 2.x (https://numpy.org/doc/stable/release/2.0.0-notes.html#numpy-2-0-python-api-removals). We specified float64 already in the line right above this.
Older documentation (https://numpy.org/doc/1.26/reference/arrays.scalars.html#numpy.float_) refers to it as an alias of double
, so I don't think this would cause any behavioral differences.
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.
LGTM
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.
@sfc-gh-joshi let's revert @sfc-gh-azhan 's workaround here ee410eb.
also, let's add some backward compatibility testing. Thanks!
3d942b7
to
97c03dc
Compare
@sfc-gh-yzou Reverted, I'll add the extra tests we discussed on slack in a followup PR. |
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1487311
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This PR fixes issues that are raised from the lint environment resolving numpy to 2.0.0. This does not mean Snowpark pandas supports numpy 2.0.0 (though Snowpark Python + local testing should); this PR just makes non-impactful code changes to pass lint and make code consistent.
Notably, numpy 2.0.0 removes
np.float_
in favor ofnp.float64
, andnp.NaN
in favor ofnp.nan
.