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

Replace rcond with rtol in pinv() call #156

Merged
merged 6 commits into from
Jul 10, 2024
Merged

Conversation

gmloose
Copy link
Collaborator

@gmloose gmloose commented Jul 9, 2024

The pinv() function deprecated the use of rcond in favour of rtol, and SciPy 1.14.0 removed the keyword option rcond completely from pinv(). Replaced all occurrences of rcond in calls to pinv() with rtol.

The `pinv()` function deprecated the use of `rcond` in favour of `rtol`, and SciPy 1.14.0 removed the keyword option `rcond` completely from `pinv()`.
Replaced all occurrences of `rcond` in calls to `pinv()` with `rtol`.
@gmloose gmloose self-assigned this Jul 9, 2024
@gmloose gmloose requested a review from darafferty July 9, 2024 20:30
gmloose added 5 commits July 9, 2024 22:36
Need to fix workflow as well, since it still used the old-fashioned way of installing (i.e. `python setup.py install`, instead of `pip install .`)
Package `python-casacore` is not (yet) compatible with NumPy 2.0. Use latest NumPy 1.x version.
Wrong tests were called.

Just invoking `pytest` should do the right thing. To be fixed in another PR.
The name of the (single) test script doesn't adhere to `pytest` standards, so it is not found.
At the same time, there are a few Python files named "test*.py" that contain currently failing tests.
This needs to be fixed in a different PR.
Use latest major version of `actions/setup-python`
Copy link
Collaborator

@darafferty darafferty left a comment

Choose a reason for hiding this comment

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

Ah, I think I have seen warnings in the past about the rcond to rtol change (but of course never did anything about it :) ). Nice that it was a fairly simple fix.

@gmloose gmloose merged commit 1f3c605 into master Jul 10, 2024
4 checks passed
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.

2 participants