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

& fkiraly [MNT] bump numpy version bound to <1.25 and fix compatibility issues #3907 #3915

Merged
merged 9 commits into from
Dec 20, 2022

Conversation

aquemy
Copy link
Contributor

@aquemy aquemy commented Dec 9, 2022

Fixes #3907

  • moves upper bound for numpy to 1.25 (1.24 is out)
  • fixes some compatibility issues due to some high precision numbers in doctests (by @fkiraly)

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 10, 2022

why are these failing? Seems like failure from numerical errors? It is numpy related.

@jorenham
Copy link
Contributor

jorenham commented Dec 10, 2022

@fkiraly It looks like a classic case of rounding errors
https://github.com/sktime/sktime/actions/runs/3655740138/jobs/6180108575#step:7:470

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 11, 2022

yes, but it did not happen before? What changed?

This produces different values on windows than on unix??

I would suggest to remove the printout from doctest and separately test until some lower amount of significant digits.
Seems to be a numerical accuracy issue.

@fkiraly fkiraly added the maintenance Continuous integration, unit testing & package distribution label Dec 11, 2022
@aquemy
Copy link
Contributor Author

aquemy commented Dec 12, 2022

I am bumping the upper bound to 1.24 to test is this was not introduced later.

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 12, 2022

yes, looks like 1.23

It also looks like it is only about the printout (number of significant digits printed), my preferred approach would be moving the tests to pytest and testing approximate equality rather than precise equality of the printout.

Would you like to do that? Otherwise I can do that too, it should not take too long.

@aquemy
Copy link
Contributor Author

aquemy commented Dec 13, 2022

I am afraid I will not have time to do it this week so feel free to take over.

@jorenham
Copy link
Contributor

numpy 1.24 was just realeased FYI
https://github.com/numpy/numpy/releases/tag/v1.24.0

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 18, 2022

@jorenham, yes, I was going to fix this and release 0.15.0 with numpy and sklearn compatibilities.

@fkiraly fkiraly changed the title [MNT] Update numpy version requirement #3907 & fkiraly [MNT] bump numpy version bound to <1.25 and fix compatibility issues #3907 Dec 18, 2022
@fkiraly
Copy link
Collaborator

fkiraly commented Dec 18, 2022

FYI @RNKuhns, the compatibility issues came from your old doctests!

Not to blame anyone, just a learning moment for me as well - looks like some numerical routines subtly changed in numpy, and you put a lot of digits after the comma in the expected doctest output.

I fixed that by moving the test out to pytest and increasing the floating point tolerance for an equality check.

@fkiraly fkiraly merged commit 0442f46 into sktime:main Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Loosen the restrictions on the numpy version requirement
3 participants