Migrate to pyproject.toml #168
Conversation
|
tests/citeproc-test.py gets run separately since it manages what errors are new. Line 19 in 2aeb250 I don't know why nosetest was used. If pytest works it might be worth switching. I've tested this locally and it seems to work fine. I've tagged this for a minor release, but will leave the PR open for at least a week in case there is additional feedback. |
This makes the project more accessible to users expecting a PEP517-formatted repository. Moreover, build tools are starting to deprecate old configuration mechanisms, see eg [1]. [1]: pypa/pip#6334
|
OK, I updated the URL, fixed some stylistic issues, and moved everything to pytest. Everything should still Just Work, and even better, we need less config.
(Except I forgot to add pytest-cov to tox.ini and am afk, will fix that tomorrow)
|
5b5e02c to
23715d8
Compare
|
Hm, actually I seem to have broken |
The tests all work unchanged, and pytest's autodetection already picks up on all of them with no need for configuration. Also, since tests/citeproc-test.py isn't named test_*.py, it is automatically ignored. Preferring pytest over nose given that it's more maintained nowadays. Added pytest-cov to automatically run coverage for us.
|
OK, it seems the combination of invoking as |
There was a problem hiding this comment.
I'm a bit nervous about the error OSError: Error reading file '/Users/runner/work/citeproc-py/citeproc-py/citeproc/data/schema/csl.rng': failed to load "/Users/runner/work/citeproc-py/citeproc-py/citeproc/data/schema/csl.rng": No such file or directory`. This should be generated by setup.py. Though there seems to be another missing file error, so maybe it's just something about the current directory. In any event the tests need to pass in order to merge.
|
Of course, investigating... |
|
Oddly enough, when I run the actions on |
|
BTW, the |
|
I don't get errors building in a chroot, either |
|
Wait! Isn't that path part of a submodule? |
|
Ahah! It appears the custom build hooks are meant to generate that |
|
... also, why on earth is |
|
That's the way it was implemented a decade ago? Way before my time.... The submodules that are part of the code are pretty fixed, while the tests get updated more often. And the approach in the tests is honestly a bit more automated and readable (and avoids confusion like #172) |
|
Not sure I agree, but that's an issue for another PR. Found the proximal cause for the breakage -- it's my dropping of |
|
Didn't manage to finish this tonight, and I'll be busy over the weekend. Made a little progress in understanding the problem:
IIUC it seems tox is installing the wheel to a venv without running the custom build hooks (possibly it's picking up the wheel from a cache and then the repository gets restored to a pristine state?) The important implication of this is that the rng file does exist inside the installed package, but not in the working directory.
Therefore, I'd recommend we use importlib_resources to look up the csl data files we package, which _should_ work regardless of the state of the working directory (since those lookups will target the package installed in the venv). With that, we should be able to have the code be more robustly location-independent.
The changes to most of the code look relatively simple. However, one corner where I still haven't figured out a way to avoid path computations is in setup.py, though perhaps that isn't so bad (and may be conceptualized as being akin to a Makefile).
|
|
I'm supportive of the move to importlib_resources to make pathing more consistent. I'm also fine treating setup.py as a special case. Also no rush! Have a good weekend. |
This makes the project more accessible to packagers used to PEP517-formatted repositories. Moreover, build tools are starting to deprecate old configuration mechanisms, see eg 1.
(Two things that struck me while testing this out -- we don't appear to be using any
nosetest-specific features, so I'm surprised we're not just using the ubiquitouspytestinstead (tests indeed work with it). Also, I'm unsure whattests/citeproc-test.pydoes, and whether it's meant to be executed as a standard test.pytestignores it by default)Note: One significant change is that
/bin/csl_unsortedhas been made a submodule ofciteproc(I've created aciteproc.scripts.*namespace for these) so as to use standard entry points, which should be more portable across platforms.Also, this incorporates #167 since I was already working on those bits
Closes: #165, #167