Skip to content
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

run doctests #2605

Merged
merged 65 commits into from Nov 3, 2023
Merged

run doctests #2605

merged 65 commits into from Nov 3, 2023

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Aug 8, 2023

This PR adds the --doctest-modules flag to pytest and makes changes necessary to allow the doctests to run.

Those changes include:

  • Fixing verbosity
  • using pytest-doctestplus to skip doctests entirely or when their dependencies are absent
  • fixing broken doctests (the bulk of the changes)

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #2605 (f2c2a52) into master (ec7f925) will increase coverage by 1.01%.
Report is 7 commits behind head on master.
The diff coverage is 82.39%.

❗ Current head f2c2a52 differs from pull request most recent head ad29ca5. Consider uploading reports for the commit ad29ca5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2605      +/-   ##
==========================================
+ Coverage   71.99%   73.01%   +1.01%     
==========================================
  Files         107      109       +2     
  Lines       11914    12038     +124     
==========================================
+ Hits         8577     8789     +212     
+ Misses       3337     3249      -88     
Files Coverage Δ
scanpy/cli.py 93.50% <100.00%> (ø)
scanpy/datasets/_datasets.py 85.71% <100.00%> (+16.18%) ⬆️
scanpy/external/pl.py 49.21% <100.00%> (+0.39%) ⬆️
scanpy/external/pp/_harmony_integrate.py 81.81% <100.00%> (ø)
scanpy/external/pp/_hashsolo.py 89.38% <100.00%> (ø)
scanpy/external/pp/_magic.py 86.11% <100.00%> (+0.39%) ⬆️
scanpy/external/pp/_scanorama_integrate.py 86.95% <100.00%> (+0.59%) ⬆️
scanpy/external/tl/_harmony_timeseries.py 28.00% <100.00%> (+3.00%) ⬆️
scanpy/external/tl/_palantir.py 25.00% <100.00%> (+2.41%) ⬆️
scanpy/external/tl/_phate.py 35.00% <100.00%> (+3.42%) ⬆️
... and 33 more

... and 4 files with indirect coverage changes

@flying-sheep flying-sheep added this to the 1.10.0 milestone Aug 8, 2023
@flying-sheep flying-sheep marked this pull request as ready for review August 10, 2023 11:28
@flying-sheep
Copy link
Member Author

It’s unblocked again apparently!

@flying-sheep
Copy link
Member Author

flying-sheep commented Oct 13, 2023

@ivirshup any way to force Azure to clear its cache or use a different runner? The “invalid instruction” error here probably comes from using a binary wheel compiled for a newer CPU.

/edit: wow, 9 attempts. Maybe just dropping Python 3.8 will get us there.

@Zethson
Copy link
Member

Zethson commented Oct 16, 2023

Scientific Python even deprecated Python 3.9 this month. Kill 3.8 please...

@flying-sheep
Copy link
Member Author

For sure, but it won’t help with this issue, I had to merge #2687 for that.

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ready for review? I think it mostly looks good.

drop 3.8

If our dependencies have dropped support we can too.


Have you thought about doctestplus in the context of this PR?

scanpy/testing/_pytest/marks.py Outdated Show resolved Hide resolved
scanpy/tests/conftest.py Outdated Show resolved Hide resolved
@flying-sheep
Copy link
Member Author

If our dependencies have dropped support we can too.

You mean like anndata? 😆

Is this ready for review? I think it mostly looks good.

yeah!

@flying-sheep
Copy link
Member Author

@ivirshup ping

@flying-sheep flying-sheep self-assigned this Oct 31, 2023
@flying-sheep
Copy link
Member Author

flying-sheep commented Oct 31, 2023

TODO: check out doctest-plus, if it helps enough, switch to it, then merge

@flying-sheep
Copy link
Member Author

OK, doctest-plus can’t even handle relative imports, and doesn’t respect --import-mode.

It needs quite some work before it works for us.

@flying-sheep flying-sheep enabled auto-merge (squash) November 2, 2023 17:27
@flying-sheep flying-sheep merged commit abbee76 into master Nov 3, 2023
13 checks passed
@flying-sheep flying-sheep deleted the doctests branch November 3, 2023 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants