StingraySoftware / stingray

Anything can happen in the next half hour (including spectral timing made easy)!
https://stingray.science/stingray
MIT License
166 stars 137 forks source link

MAINT: use new location for dev wheels #785

Closed bsipocz closed 6 months ago

bsipocz commented 6 months ago

The location for the dev wheel has changed so in fact you haven't been testing with e.g. dev numpy.

I would expect this PR will uncover some numpy 2.0 incompatibilities, unfortunately I won't be able to follow-up in a timely manner to fix any of those, please feel free to push to this PR.

Also, I see a lot of opportunity to cleanup the CI scripts to drop unused variables, old versions, LTS, etc. That is also beyond the scope here.

And python 3.12 is also out in a while, so I would think it would worth thinking about formally adding it to the CI (and fix any incompatibilities)

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (753eb2d) 96.30% compared to head (68f073b) 78.26%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #785 +/- ## =========================================== - Coverage 96.30% 78.26% -18.04% =========================================== Files 43 43 Lines 8473 8495 +22 =========================================== - Hits 8160 6649 -1511 - Misses 313 1846 +1533 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

matteobachetti commented 6 months ago

Excellent catch @bsipocz! Fortunately, there are no incompatibilities with new numpy versions. Indeed, we have some dust to remove from our CI scripts, I'll take a shot in the next few days to see if I can add to this PR and merge quickly. I might ask you to take a look at the modifications to make sure I'm not doing anything stupid 😅

bsipocz commented 6 months ago

I'll be offline for the holidays, but if you tag me in a pr, I'll have a look when I'm back in January.

bsipocz commented 6 months ago

Oh, and looking at the log, unfortunately it looks like one of the dependencies upper limited numpy, so 2.0 hasn't actually been tested

bsipocz commented 6 months ago

OK, now the failures are indeed coming from stingray, so I'm handing it off.

pep8speaks commented 6 months ago

Hello @bsipocz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2023-12-27 14:22:39 UTC
matteobachetti commented 6 months ago

@bsipocz one remaining issue seems to be internal to Yaml, and maybe workable around from Astropy. I opened an issue here: https://github.com/astropy/astropy/issues/15792

I had a lot of docstring issues from a change of representation of values in Numpy 2.0, like:

091     >>> get_total_gti_length(gti)
Expected:
    1021
Got:
    np.int64(1021)

The rest of issues should be solved. Thanks a lot for catching the issue, and allowing much more robust testing!

matteobachetti commented 6 months ago

I think it's ready to be merged now. Since this is important to properly test ongoing PRs, I'm going to merge soon. Thanks again @bsipocz!