-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
MNT: Infrastructure and other updates #202
Conversation
3e10373
to
6b14578
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #202 +/- ##
==========================================
+ Coverage 78.83% 81.32% +2.49%
==========================================
Files 10 10
Lines 992 996 +4
==========================================
+ Hits 782 810 +28
+ Misses 210 186 -24 ☔ View full report in Codecov by Sentry. |
Replace broken URL and deprecated np.trapz usage. Ignore link that is blocking CI check.
0d5330b
to
9c9418d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
@@ -1,21 +1,16 @@ | |||
[tox] | |||
envlist = | |||
py{310,311,312}-test{,-devdeps,-predeps}{,-cov} | |||
build_docs | |||
py{38,39,310,311,312}-test{,-alldeps}{,-oldestdeps,-devdeps,-predeps}{,-cov} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have discussed and agreed that we should require >=3.10 so that we can use the improved type annotation capabilities. this change is still staged in #182, but it would make more sense to wrap that into this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even numpy has not dropped Python 3.9 yet. Are you sure you want to do it so soon?
https://github.com/numpy/numpy/blob/c9340c210666afefb6946fa6f3a105856ff0e418/pyproject.toml#L19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, numpy has a much, much larger installed base to support and we don't. 3.10 isn't all that new so it's not a huge ask. if one cares at all about performance, they really want 3.11+, anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is so awesome! so many cleanups and fixes i've wanted to see, but never got around to. the migration to pyproject.toml
is also extremely welcome. my only niggle is that we made the decision on slack 7 months ago to set python_requires
to >=3.10
. this was implemented in #182, but not yet merged into main
. i'm fine with merging this as-is and rebasing/revising #182 accordingly. otoh, this is probably the more sensible PR to make that change given the rest of the changes its making to package config/infrastructure.
Sorry about the conflict with #182 . I don't watch this repo so I was not aware of it. I replied in #202 (comment) (I think it is a little too soon but I am also not a specreduce maintainer). Whether you want to merge this first, or cherry-pick your changes to this PR and take this over, I will leave to you. |
I don't have any strong opinions on the order of operations - good to merge on my end! Thanks for all the improvements @pllim ! |
Then, feel free to merge! I am tempted to merge myself but I shouldn't. Thanks! |
I stared into the code and I couldn't unsee things, so here it is.
--remote-data
flag in any of your CI job.Unable to handle this one until I get access:After merge, can add new job names to branch protection.