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

TST: Skip Cython test for editable install #26325

Merged
merged 4 commits into from Apr 23, 2024

Conversation

thalassemia
Copy link
Contributor

Skip testing Cython extension for editable installs due to issues identified in scipy/scipy#20540.

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

See also cython/cython#5820 for a similar discussion.

This LGTM, I am not 100% sure if there may be a better way to detect "editable", but this also seems fine to just use.

This doesn't remove the pain that you may not be able to develop downstream cython modules if you install NumPy with an editable install. But there isn't much to do about that...

Also make it work with meson-python 0.15.0 (that returns an empty list
for `__path__`).
@rgommers rgommers self-requested a review April 23, 2024 08:11
@rgommers rgommers added this to the 2.1.0 release milestone Apr 23, 2024
The main reason is that tests with a compile step cannot work,
because headers aren't found.

The distutils test could be made to work, but that isn't worth
the effort since it's deprecated code.
@rgommers
Copy link
Member

Thanks @thalassemia! I tweaked the check for editable install a bit to ensure it works with meson-python <0.16.0, and then used it to mark all other failing tests. This was never complete it looks like. Now pytest numpy passes.

Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

Should we maybe add a smoketest run with an editable build of numpy?

pytest.ini Show resolved Hide resolved
@rgommers
Copy link
Member

rgommers commented Apr 23, 2024

Should we maybe add a smoketest run with an editable build of numpy?

Yes we definitely should. I'd prefer to open a follow-up issue for that though, since making CI happy takes time/iterations.

@ngoldbaum
Copy link
Member

SGTM, merging. Thanks @thalassemia!

@ngoldbaum ngoldbaum merged commit ef5f10d into numpy:main Apr 23, 2024
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants