"Modernize" GitHub Wrokflows #18
Conversation
… generate_autosummary_content(), who's signature changes for sphinx >= 8.0
rocco8773
added
CI
namurphy
left a comment
namurphy
left a comment
There was a problem hiding this comment.
Good idea splitting this out! It's looking good so I approved it, but let me know if you'd like me to take a look at this again. I have a few thoughts and minor suggestions.
| if sys.version_info < (3, 8): # coverage: ignore | ||
| raise ImportError("plasmapy_sphinx does not support Python < 3.8") | ||
|
|
There was a problem hiding this comment.
| if sys.version_info < (3, 8): # coverage: ignore | |
| raise ImportError("plasmapy_sphinx does not support Python < 3.8") |
plasmapy_sphinx is likely only going to be used by experienced developers and 3.8 has been around for a 5 years, so my inclination would be to drop this.
There was a problem hiding this comment.
Yeah, we should probably do that. I didn't since I'm using plasmapy_sphinx on packages that have to support older python versions, and I just want to make sure it's a non-issue before dropping support here.
In reality, I can restrict building documentation to newer versions of python for those packages that have to support older version of python.
| fetch-depth: 0 | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
Thank you for learning from my mistakes! 😅
.github/workflows/check_install.yml
Outdated
| fetch-depth: 1 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v4 |
There was a problem hiding this comment.
| uses: actions/setup-python@v4 | |
| uses: actions/setup-python@v5 |
This might need to be changed elsewhere too.
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
| uses: actions/checkout@v3 | |
| uses: actions/checkout@v4 |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python: ['3.8', '3.10', '3.11', '3.12', '3.13'] |
There was a problem hiding this comment.
🌠 If we wanted to be ambitious and thorough later on (not in this PR!), the lowest-direct resolution strategy of uv would let us test plasmapy_sphinx against the oldest allowed versions of direct dependencies. 🤔
There was a problem hiding this comment.
Future work... I just want to get this to the next step first.
|
@namurphy Thanks for the quick review...I didn't even have to request it. :) |
This PR updates the GitHub workflows associated with
plasmapy_sphinx. This was need due to infrequent updates toplasmapy_sphinxand changes made tosphinx. What was done...3.6to3.8packagingplasmapy > 8.0check_install.ymlwas created to test installing and importingplasmapy_sphinxonubuntu-latestfor python3.8,3.10,3.11,3.12, and3.13documentation.ymlworkflowdocumentationto test on python3.8,3.10,3.11,3.12, and3.13documentation.ymlworkflowsphinx_limitsto test documentation building onsphinx < 7.2,sphinx < 7.4andsphinx < 8.2. Changes in thesesphinxversion were not backwards compaitble.sphinx >= 8.2broke functionality related togenerate_autosummary_content(). The change dropped argumentappfor required keywordsconfig,events, andregistry.plasmapy_sphinxadopted a strategy to pass the expected arguments based on the installedsphinxversion.sphinx >= 7.4broke functionality related toimport_by_name(). The functionality changed how it was handling prefixes for the modules. I updatedAutomodsumm.import_by_name()to automatically remove an items prefix from its name before callingAutosummary.import_by_name().sphinx >= 7.2refactored theautodocoption:noindexto:no-index:. So ourautomodapifunctionality can handle the more modern:no-index:, I updatedModAPIDocumenterto set the:noindex:option if:no-index:was given when usingsphinx < 7.2to build.Note
This PR originated as PR #17, but that PR scope creeped. Thus, the workflow functionality from #17 was broken out into this PR.