| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
On my machine/setup (macOS, Python 3.9.13) tests pass on the main branch, so it's probably not some package update... |
Sorry, something went wrong.
|
Ah, no, that was not the template test but the test from the generated package... |
Sorry, something went wrong.
|
Ok, it does seem to be some kind of update, because the failures happen on main now as well: https://github.com/egpbos/python-template/actions/runs/6106092589 |
Sorry, something went wrong.
|
The problem originates with this Sphinx 7.2 functionality which was added since August: https://www.sphinx-doc.org/en/master/usage/extensions/coverage.html#confval-coverage_statistics_to_report No idea why it doesn't generate this in 3.7 and 3.8... |
Sorry, something went wrong.
|
Maybe the new version is marked as requiring 3.9 or higher, so that if you pip install Sphinx on 3.7 or 3.8 you get an older version that doesn't do this? |
Sorry, something went wrong.
|
@LourensVeen you nailed it! https://github.com/sphinx-doc/sphinx/blob/fcc38997f1d9b728bb4ffc64fc362c7763a4ee25/pyproject.toml#L16C26-L16C26 Ok, so I would say this is a separate issue (i.e. we don't have to block this PR over it), but just to start the discussion: how should we handle this? Just skip the test_coverage_api_docs test for Python < 3.9? Would be as easy as adding a line like this: https://docs.pytest.org/en/7.1.x/how-to/skipping.html#id1, so maybe it could actually be done in this PR just to keep things simple. |
Sorry, something went wrong.
|
I updated the test in line with @egpbos 's comment. This means we are not testing for documentation coverage at all, for versions before 3.9, which is fine I think. For reasons, I created a separate PR which I merged into this one myself. Ask me about it at the water cooler ;-) Of course, writing a test was more complicated than expected, as lines in the report switch order between runs. |
Sorry, something went wrong.
|
Does it really take almost 10 minutes to run the tests on a project with barely any content? |
Sorry, something went wrong.
|
@bouweandela, On Ubuntu about 3-4 minutes and 8-9 minutes for mac or windows. While running the tests, about 9 times, a fresh virtual environment is created, in which a new package is generated and installed using the cookie cutter template. I have to say that the tests are actually quite useful and already helped me find errors a couple of times, which I would probably have overlooked otherwise. Note that this PR is about reducing the computation time needed for linting the generated package. It hardly reduces the testing time of the template (this repo). |
Sorry, something went wrong.
|
10 minutes sounds normal, unfortunately. Installing dependencies is slow. |
Sorry, something went wrong.
There was a problem hiding this comment.
Looking good! Commented on issues that should be addressed, good to merge after that, I think.
Sorry, something went wrong.
I had a go at making that a bit faster in #353 |
Sorry, something went wrong.
|
I'm going to rebase this branch slightly as well. Just for your entertainment, here is what the branch looked like before: * 54fb005 minor fixes for adding prospector (HEAD -> 336-use-ruff-instead-of-prospector-and-isort, upstream/336-use-ruff-i> | * b936ef9 Merge pull request #349 from NLeSC/fix-coverage-api-docs |\ | | | * ab72989 update test to expect the output of a new version of sphinx See https://github.com/NLeSC/python-template/pull/> |/ | * 2772376 add ruff test and remove isort and prospector tests (Refs #336)) | * 1b48732 bring template code and ruff config in sync (Refs #336) | * c36c520 use ruff instead of prospector and isort (Refs #336) | * a53c275 ... Of course, that little bump over there is not UGH 😆 Also, my commit can be a squashed together with the first three commits, all four are part of one functional change. The fixed test is functionally different, so we can keep that one separate. Although, for clarity we could keep the 1b48732 bring template code and ruff config in sync commit separate for the latter reason as well; ruff checks are apparently slightly different, but it would not be immediately clear just from looking at the code changes in a combined commit that for instance those docstring changes were actual functional changes instead of just cosmetic (which often is done together with functional changes and which then often confuses some reviewers 😄 ). So, we do: git rebase -i HEAD~5 And we rewrite history into: reword ab72989 update test to expect the output of a new version of sphinx See https://github.com/NLeSC/python-template/ pull/347#issuecomment-1710684574 pick c36c520 use ruff instead of prospector and isort (Refs #336) squash 2772376 add ruff test and remove isort and prospector tests (Refs #336)) squash 54fb005 minor fixes for adding prospector pick 1b48732 bring template code and ruff config in sync (Refs #336) Note that just for fun, I also put the fixed test first and reworded it so that the "See URL" part is in the message instead of the title (there was no newline in between so that git saw it as one title). UGH! |
Sorry, something went wrong.
|
Thanks a lot for this! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Description
I replace every use or mention of 'prospector' or 'isort' with a 'ruff' equivalent. See discussion in #336.
I don't think the linting will be exactly the same as it was, but it will be good and much faster. Same goes for sorting imports.
Instructions to review the pull request
Create a python-template-test repo on GitHub (will be overwritten if existing)