Moving source directory to src/anndata

[flying-sheep]

Supersedes #1128

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.25%. Comparing base (a538b71) to head (a834c42).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1151      +/-   ##
==========================================
+ Coverage   84.22%   84.25%   +0.02%     
==========================================
  Files          36       35       -1     
  Lines        5622     5613       -9     
==========================================
- Hits         4735     4729       -6     
+ Misses        887      884       -3     

Files	Coverage Δ
src/anndata/__init__.py	92.00% <100.00%> (ø)
src/anndata/_core/access.py	72.22% <ø> (ø)
src/anndata/_core/aligned_df.py	97.77% <ø> (ø)
src/anndata/_core/aligned_mapping.py	91.96% <ø> (ø)
src/anndata/_core/anndata.py	84.83% <ø> (ø)
src/anndata/_core/file_backing.py	90.21% <ø> (ø)
src/anndata/_core/index.py	93.19% <ø> (ø)
src/anndata/_core/merge.py	82.92% <ø> (ø)
src/anndata/_core/raw.py	79.28% <ø> (ø)
src/anndata/_core/sparse_dataset.py	94.72% <ø> (ø)
... and 25 more

... and 1 file with indirect coverage changes

[flying-sheep]

The issue is that doctests make the testing process wonky, as they import their parent modules.

And if we don’t use editable installs in testing, the parent modules are the one in the project directory while there’s a second copy installed to the site-packages directory.

Using src layout, editable installs are fine for testing, as the src directory gets added tosys.path, not the project directory. And the src directory should be clean while the project directory potentially contains a bunch of stuff that python interprets as modules.

A different approach would be to do regular installs and run the doctests for the installed version after moving the tests out of the package. Like pytest --config=pyproject.toml ./tests /.../site-packages/anndata

And for completeness’s sake, there’s also PY_IGNORE_IMPORTMISMATCH=1

[flying-sheep]

This can be finished once the fix to this has been released: pytest-dev/pytest#11475

I.e.: pytest-dev/pytest#12074

[flying-sheep]

OK, I reverted the commit that fixes the double collection.

We have two options:

1. testpaths = ['anndata', './src/anndata/tests', ...]

   this works with regular installs, since we remove the tests from the installed package.
   in editable installs, this will collect and run unit tests twice (until we move them out of the package)

2. testpaths = ['anndata', ...]

   this works with editable installs, where tests are inside the package.
   in regular installs, this will only run doctests, not unit tests.

of course that’s fixable by not relying on testpath on one side: E.g. choosing 2. and explicitly runningpytest anndata ./anndata/tests ... in CI works.

once we finally move the tests out of the package, everything can be unified again: testpaths = ['anndata', './tests'] will work for both kinds of installation.

[ivirshup]

@flying-sheep is this ready for me to look at again?

[flying-sheep]

Of course

[flying-sheep]

still the case

[ivirshup]

@flying-sheep, is it still the case that test collection doesn't really work with pip install ., but does with pip install -e .? That's what I see.

Do you know why this is different for CI?

[ivirshup]

Some explanation of why, link to a relevant issue here would be good

[flying-sheep]

because you said so. there’s no issue that I’m aware of.

You said something along the lines of “let’s not make too many changes at once because this is fragile. Let’s do that when we move the tests out of the package.”

[flying-sheep]

Do you know why this is different for CI?

because we directly specify paths for CI.

you agreed that this is fine for now, since the problem will fix itself once we move tests out of the package

[flying-sheep]

OK, I check if we run enough tests and link the issue.

[ivirshup]

@flying-sheep just a heads up that CI is failing here

[ivirshup]

lgtm

Once it's merged maybe send out a message on zulip with instructions on how to

• Update a PR
• switch over to this (I think just re-install and rm -r anndata?)

[ivirshup]

Also, still planning on back porting this? I'm going to lunch now, but wanna make a release before that.

[flying-sheep]

doesn’t matter if it’s in the release or not, but it should be backported to the 0.10.x branch so further backports are easier.