DOC/ENH: distinguish missing packages from installed ones with incompatible versions in docs/conf.py - #20414
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
46672fc to
47cc72f
Compare
47cc72f to
3e061f6
Compare
| required=req.specifier, | ||
| ) | ||
| ) | ||
| continue |
There was a problem hiding this comment.
This is an actual bugfix I found while self-reviewing the PR: not having a continue in the original code could lead to a NameError if the first package tried wasn't found (version would be unbound), and to duplicate lines if a package wasn't found and its version requirement was not satisfied by the version of the latest found package...
|
It looks good. But is there any way to test this? Testing conf.py files within the test env they set up is kinda hard... |
3e061f6 to
50054d9
Compare
|
It's actually pretty trivial to test locally, just install sphinx via pip within a fresh env then run |
…atible versions in `docs/conf.py`
50054d9 to
5681655
Compare
|
This is getting a bit complex and would also be useful to have for coordinated packages - I'm curious if you have any thoughts about how we could potentially factor this out? For instance, we could always have a helper function in sphinx_astropy then have: Yes it's still not a single line, but it would save quite a bit of code over a number of repos. |
|
I don't know if it'd fit sphinx-astropy's scope, but I'd be happy to move the correct version of it there if you think it makes sense. |
nstarman
left a comment
There was a problem hiding this comment.
LGTM.
I'd also be happy generalizing this tool out of Astropy.
.... I'd love to do that for more of Astropy's non-core functionality!
|
@astrofrog to be clear, should be proceed with this PR as is and then upstream the function ? |
|
Yes let's not hold this up |
Description
Triggered by this discussion, I asked myself how to better leverage
packagingAPIs in this for loop, and went down a rabbit hole. I didn't intended to open a PR at first, as I was doing it for fun/training (I won't bill the time I spent there), but I ended up with one real (albeit minor) improvement: now, packages that are found but incompatible with requirements are explicitly reported as such, instead of being reported as plainly missing.This also includes the small change that triggered the discussion about canonicalizing package names.
AI Disclosure
None
Merge method