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.

Sign up for GitHub

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

Jump to bottom

Bump cython, numpy #105

Merged
merged 7 commits into from
Sep 26, 2024
Merged

Bump cython, numpy #105

merged 7 commits into from
Sep 26, 2024

Conversation

nauaneed
Copy link
Contributor

No description provided.

@pep8speaks
Copy link

pep8speaks commented Aug 18, 2024 •
edited
Loading

Hello @nauaneed! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 277:28: E261 at least two spaces before inline comment
Line 330:28: E261 at least two spaces before inline comment

Line 501:80: E501 line too long (80 > 79 characters)

Comment last updated at 2024-09-25 15:52:21 UTC

prabhuramachandran
prabhuramachandran requested changes Aug 18, 2024
View reviewed changes
Copy link
Contributor

@prabhuramachandran prabhuramachandran left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Can you please bump the Python version to at least 3.11 or 3.12 so the tests run on latest Python.

@nauaneed nauaneed force-pushed the bump-cython branch 2 times, most recently from f67e665 to 37868c5 Compare September 20, 2024 20:54
nauaneed
nauaneed commented Sep 20, 2024
View reviewed changes
compyle/types.py
@@ -188,6 +188,7 @@ def _inject_types_in_module():
NP_C_TYPE_MAP[np.dtype(np.uint64)] = 'unsigned long long'
C_NP_TYPE_MAP['long long'] = np.int64
C_NP_TYPE_MAP['unsigned long long'] = np.uint64
TYPES['long long'] = KnownType('long long')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes tests failing on windows but not sure if this is a good fix. All keys in the TYPES dict do not contain spaces as they are meant to be used with @annotate decorator?

@nauaneed
Copy link
Contributor Author

nauaneed commented Sep 20, 2024 •
edited
Loading

One failing test on Windows remaining: test_ext_module.py::TestExtModule::test_that_multiple_writes_do_not_occur_for_same_source. This is Python version dependent. The failure happens on 3.10.[0-10] and 3.11.[0-10] but not on 3.9.[0-10] and 3.12.[0-5]

@nauaneed
fix(tests): cmp radix sort with stable argsort
8d065a3 
As radix sort is stable, we need to enshure that that the argsort
result it is compared with is also stable.

Fixes pypr#104
@nauaneed
fix(types): add "long long" as a KnownType on win
aada2af 
tests/test_array.py::test_comparison was failing on windows without this
@nauaneed
chore(deps): build using numpy>2.0
bca0e31 
Ref https://github.com/scipy/oldest-supported-numpy/blob/687ffc2f57e29ec7c74a28c58c247d16d0f6bee1/README.rst
prabhuramachandran
prabhuramachandran approved these changes Sep 26, 2024
View reviewed changes
Copy link
Contributor

@prabhuramachandran prabhuramachandran left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! We should fix the multiple writes error by using a proper cross platform lock like fasteners. Will try to do that later.

@prabhuramachandran prabhuramachandran merged commit dd566c3 into pypr:master Sep 26, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prabhuramachandran prabhuramachandran prabhuramachandran approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nauaneed @pep8speaks @prabhuramachandran