Use import_module instead of find_spec#39423
Conversation
|
Documentation preview for this PR (built with commit ef72c43; changes) is ready! 🎉 |
tobiasdiez
left a comment
There was a problem hiding this comment.
LGTM. @tornaria is this change fine with you?
I've no idea, this code is very old. I didn't write it, only fixed it to work on python 3.12. The original code comes from from #15351 (comment). AFAICT, nothing uses this function I'm not using meson to build sagelib. For my void package I use the sagemath-standard sdist from pypi, which I don't think allows building with meson. Is there a published sdist to build sagelib with meson? |
|
@tornaria Thanks for the feedback. Tests are green (apart from the usual random failures), so let's get this in. |
sagemathgh-39423: Use import_module instead of find_spec Otherwise the test would fail with meson editable install. See https://g ithub.com/sagemath/sage/actions/runs/13003203795/job/36265539648 . Looks like the function was last changed in sagemath#36407. There was no discussion why the simple implementation is not used. This is part of the fix for this test. The other part needed is sagemath#39498 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. (can't really test, but see sagemath#39369) - [ ] I have updated the documentation and checked the documentation preview. (no documentation change) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39423 Reported by: user202729 Reviewer(s): Tobias Diez
sagemathgh-39369: Test pip editable install with meson Tests pip editable install with meson on GitHub Actions. This uncovers several failing tests, which are fixed by (see dependencies below) Note: review is hard because of the large number of dependencies. It is probably easier to look only in `.github` folder. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. ### ⌛ Dependencies - sagemath#39423 - sagemath#39498 - sagemath#39494 - sagemath#39424 - sagemath#39275 - sagemath#39499 URL: sagemath#39369 Reported by: user202729 Reviewer(s): Tobias Diez, user202729
Otherwise the test would fail with meson editable install. See https://github.com/sagemath/sage/actions/runs/13003203795/job/36265539648 .
Looks like the function was last changed in #36407. There was no discussion why the simple implementation is not used.
This is part of the fix for this test. The other part needed is #39498
📝 Checklist
⌛ Dependencies