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

[Bug]: _path.is_sorted is wrong for the non-native byteorder case #25995

Closed
anntzer opened this issue May 28, 2023 · 2 comments · Fixed by #27595
Closed

[Bug]: _path.is_sorted is wrong for the non-native byteorder case #25995

anntzer opened this issue May 28, 2023 · 2 comments · Fixed by #27595
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented May 28, 2023

Bug summary

_path.is_sorted always reads data from the buffer in native byteorder, thus it can give incorrect results when the input is in non-native byteorder. (This is also true for is_sorted_and_has_non_nan in #25978.)

Code for reproduction

# The array below reads as [2, 256] after byteswapping.
mpl._path.is_sorted(np.array([33554432, 65536], ">i4"))

Actual outcome

True

Expected outcome

False

Additional information

We don't actually really need to support any case other than native-order floats in is_sorted (because we only ever call it with a freshly constructed float array), so we should just restrict support to that case.
It may also be useful to inspect the other C APIs which may likewise be impacted by wrong byteorderness.

I also doubt that the speed gain from having a specialized C implementation of is_sorted (compared to the plain numpy nanmask = np.isnan(x); x_finite = x[~nanmask]; x_finite.size and (x_finite[1:] >= x_finite[:-1]).all() or similar -- note that we already compute nanmask and x_finite below) is really worth the trouble.

Operating system

macos

Matplotlib Version

3.8.0.dev1128+g5438e94fa7

Matplotlib Backend

any

Python version

3.11

Jupyter version

No response

Installation

git checkout

@tacaswell
Copy link
Member

I am curious how you found this issue.

@tacaswell tacaswell added this to the v3.8.3 milestone Jan 3, 2024
@anntzer
Copy link
Contributor Author

anntzer commented Jan 3, 2024

I don't really remember, but I suspect that was simply by staring at the code while trying to fix the bug underlying #25978...

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 a pull request may close this issue.

2 participants