-
Notifications
You must be signed in to change notification settings - Fork 183
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
Changes for numpy 2.0 to fix test matrix. #1196
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1196 +/- ##
==========================================
- Coverage 87.44% 87.33% -0.12%
==========================================
Files 113 113
Lines 10238 10281 +43
Branches 4059 4065 +6
==========================================
+ Hits 8953 8979 +26
- Misses 691 706 +15
- Partials 594 596 +2 ☔ View full report in Codecov by Sentry. |
Seem like I gave one more set of changes before this is complete. Not sure why onle one version triggered it. |
I am unable to replace the error seen in the test suite but it most likely is a Python behavior not a JPype one. The test was making sure that all cast types go through required paths. Python itself through the exception that "e" type is not a simple data type. If they choose to change that behavior then we shouldn't test against it. @marscher I believe this is ready to go. |
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.
Not much to this other than changes to the test bench to get it back on line.
The only odd code was to better support the half float which is valid buffer type in Python. There is no native Java type for it.
|
||
namespace | ||
{ | ||
|
||
template <jvalue func(void *c) > |
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.
This is a crude implementation of the "half" floating type. There is no native type in Java to support this but it is possible that someone could pass a half floating in for promotion to either float or double. This code is exercised in the test suite.
@@ -385,6 +436,31 @@ jconverter getConverter(const char* from, int itemsize, const char* to) | |||
case 'd': return &Convert<double>::toD; | |||
} | |||
break; | |||
case 'e': | |||
if (reverse) switch (to[0]) |
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.
I am not sure if there is every a case in which a reversed byte order half floating value would be converted. Through the order of this operation is reasonable to define. Reverse the bytes, promote to float then convert to Java type.
Thanks. LGTM |
This PR deals with the broken test matrix following the removal for
float_
. I replaced it withfloat16
though that is actually a different test. I dealt with the required alterations to supporthalf
type for array conversion, though I can't find a good pattern to deal with byte reversed arrays so that may be untested.