Pin matplotlib in oldestdeps job #1015
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1015 +/- ##
=======================================
Coverage 70.03% 70.03%
=======================================
Files 64 64
Lines 4339 4339
=======================================
Hits 3039 3039
Misses 1300 1300 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Failure is unrelated. I am just going to merge this now. |
| @@ -46,6 +46,7 @@ deps = | |||
| oldestdeps: asdf-astropy==0.3.* | |||
| oldestdeps: asdf==2.10.* | |||
| oldestdeps: ndcube==2.0.* | |||
| oldestdeps: matplotlib==3.6.* | |||
There was a problem hiding this comment.
If this is really the oldest supported version (and is it?), should that be reflected in the requirements in setup.cfg?
Asking since mamba wanted matplotlib 3.5.* for numpy 1.19, and tests are still passing with 3.5.3.
There was a problem hiding this comment.
No, that is not the oldest supported version. It is actually the maxversion we can allow for matplotlib for the oldest supported version. According to setup.cfg , matplotlib is only a test dependency.
There was a problem hiding this comment.
Yes, but if installed as test dependency (and not necessarily though tox), should not that also ensure a supported version is installed? And now that a version is specified in oldestdeps, I would expect, well, the actual oldest supported version to be tested (may be 3.1.0, which is the oldest I could reliably install and test with Python 3.8, or even older); just as it is done for numpy etc.
There was a problem hiding this comment.
Well, actually the tests are running (with the same 435 passing w/o remote_data) without any matplotlib installed, as it is only ever imported in the docs. So maybe the question rather is whether it belongs into test deps at all (especially with no doctests configured).
There was a problem hiding this comment.
Pretty sure specutils runs doctest because I just fixed those.
There was a problem hiding this comment.
(there existed a build_docs job at the time when #849 was open, but I cannot see any obvious replacement for that) – ok, seeing that they are pulled in with cov, but I cannot reproduce why oldestdeps is including cov. It obviously is, so perhaps a comment would be helpful indeed.
There was a problem hiding this comment.
I think this setting pulls in test dependencies for all the jobs:
Lines 62 to 63 in 6eb7f96
There was a problem hiding this comment.
Yes, that is clear; was just wondering if anything running doctests would also pull in extras = docs as linkcheck does. Answer is 'no'; turns out I misread the
!cov: pytest --pyargs specutils {toxinidir}/docs {posargs}
command for a commented line! (Maybe one should go Spanish and use ¡ for not ;-)
There was a problem hiding this comment.
So, do you still need a comment in tox.ini?
There was a problem hiding this comment.
I have reconstructed now how it is a documentation dependency that is required for all tests; no strong opinion either way if no one else has stumbled over it.
matplotlib 3.7 bumped minversion of numpy.
Fix #1014