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

Install cysignals-CSI-helper as package data for better portability #204

Merged
merged 13 commits into from
Nov 19, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Jul 6, 2024

Rather than installing cysignals-CSI-helper.py into a share directory and then trying to figure out the correct path to it, install it as Python package data and use the standard importlib.resources API to access it. For Python versions older than 3.9, the importlib_resources backport is used instead.

Fixes #200

@orlitzky
Copy link

orlitzky commented Jul 9, 2024

Thanks for working on this.

@orlitzky
Copy link

orlitzky commented Oct 6, 2024

@mkoeppe any reason not to merge this? Without it, we guess the wrong path on Gentoo.

@dimpase
Copy link
Member

dimpase commented Nov 18, 2024

This should be updated, as we don't deal with python < 3.9 here any more. @tobiasdiez - does this fit into the meson setup?

@tobiasdiez
Copy link
Contributor

Yes, one can easily control where the script should be installed.
https://github.com/sagemath/cysignals/blob/main/src/scripts/meson.build

@dimpase
Copy link
Member

dimpase commented Nov 19, 2024

@mgorny - please take a look at the PR to move this forward: mgorny#1

@mgorny
Copy link
Contributor Author

mgorny commented Nov 19, 2024

Err, that PR doesn't apply cleanly to my branch, i.e. GH doesn't let me merge it. Feel free to just overwrite my branch (it's open to maintainers) or replace the PR.

@dimpase
Copy link
Member

dimpase commented Nov 19, 2024

Err, that PR doesn't apply cleanly to my branch, i.e. GH doesn't let me merge it. Feel free to just overwrite my branch (it's open to maintainers) or replace the PR.

while maintainers are too busy, and I'm not one, the quickest way forward would be just to force-push my branch into your PR.

@mgorny mgorny force-pushed the fix-helper-install branch from 2abd7ef to 6d36c5f Compare November 19, 2024 12:21
@mgorny
Copy link
Contributor Author

mgorny commented Nov 19, 2024

Err, that PR doesn't apply cleanly to my branch, i.e. GH doesn't let me merge it. Feel free to just overwrite my branch (it's open to maintainers) or replace the PR.

while maintainers are too busy, and I'm not one, the quickest way forward would be just to force-push my branch into your PR.

Done.

@dimpase
Copy link
Member

dimpase commented Nov 19, 2024

@vbraun - can you promote me to maintainer here, or otherwise take care of this and other PRs waiting here?

@dimpase
Copy link
Member

dimpase commented Nov 19, 2024

I haven't figured out to invoke testgdb.py from pytest. Any pointers?

@dimpase dimpase merged commit 9f39b7d into sagemath:main Nov 19, 2024
@tobiasdiez
Copy link
Contributor

I haven't figured out to invoke testgdb.py from pytest. Any pointers?

This still uses unittest. So one needs to first migrate it to pytest, and then rename it to end on _test.py so that it gets automatically collected by pytest. I'll open an issue to track this.

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 this pull request may close these issues.

Use importlib.resources to find cysignals-CSI-helper.py
4 participants