Skip to content

Review numpy API coverage #224

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

Open
wshanks opened this issue May 16, 2024 · 4 comments · May be fixed by #318
Open

Review numpy API coverage #224

wshanks opened this issue May 16, 2024 · 4 comments · May be fixed by #318

Comments

@wshanks
Copy link
Collaborator

wshanks commented May 16, 2024

Since Uncertainties has not been updated much recently, there has been time for the functions in the unumpy module to drift relative to upstream numpy. Following on from the need to look at the numpy support because of numpy 2 (#222), it would be good to review the current unumpy interface to make sure the current functions still match numpy's versions (no change in function signature) and no functions added to numpy that a user would expect are missing. Besides that there are a couple known action items:

  • numpy 2 introduced aliases for inverse trigonometric functions like acos for arccos. It would be good for unumpy to include similar aliases.
  • Review if unumpy.matrix could be updated/deprecated. The corresponding numpy.matrix has been documented as not recommended and possibly to be removed for a long time now. However, the numpy developers did not take the opportunity to remove it with numpy 2.0 and likely numpy 3.0 will not happen for a long time. Still, unumpy.matrix could perhaps migrate to deriving from a standard array instead of numpy.matrix.
@wshanks wshanks mentioned this issue May 16, 2024
@jagerber48
Copy link
Contributor

#262 here is the diff for unumpy.core. https://github.com/lmfit/uncertainties/pull/262/files#diff-e5500f0177ef7e9fab3f620bf08f3430a76a807036f28b17e574214ad79e4b4f. I need to go through and disentangle what, if any, API change I am making. I think it was easy enough to keep support for matrices, at least in some case. I would be just as happy, actually happier, to drop support for matrices, but I think it would require at least one minor versions worth of deprecation warnings.

@jagerber48
Copy link
Contributor

jagerber48 commented May 25, 2025

Ok, it's coming back to me. https://github.com/lmfit/uncertainties/pull/262/files#diff-3920e6d42d819e305be2c6e3cb5318e2bb0e7c35f62e5b25e92bd41726a869f1R165

There is a comment on this line. The way the code is now does not fully support numpy matrices. It partially supports them. Getting back support at the level of master would require some more work.

I vote for uncertainties to drop support for matrices in 4.x. I predict everyone will agree so I'm going to make a PR deprecating it in 3.x.

@jagerber48 jagerber48 linked a pull request May 25, 2025 that will close this issue
5 tasks
@wshanks
Copy link
Collaborator Author

wshanks commented May 25, 2025

I think it is good to drop matrix support (and internal usage) since numpy deprecated it. It would be nice to keep a convenience function for inverting a square array with uncertainty propagation. I think that is all we were getting from the matrix class (besides * acting as matrix multiplication which is not so important since @ was added as a dedicated symbol for that).

@jagerber48
Copy link
Contributor

Yes, the code I have now for 4.0.0 maintains a function that inverts square arrays. I think the current code can take a function that accepts one array as an argument and allow it to handle arrays with uncertainty. My new code supports code with an arbitrary number of arrays as inputs and it will detect with have uncertainty and give the appropriate result. I don't know if this generalization induces a performance hit though...

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 a pull request may close this issue.

2 participants