search

a-renzini / pygwb

MIT License
3 stars 3 forks source link

[JOSS Review] `pytest` is not installed #5

Closed Sbozzolo closed 9 months ago

Sbozzolo commented 12 months ago

The documentation asks to run tests with pytest, but the package is not available when installing pyGWB in a clean environment with the method described in the docs.

Sbozzolo commented 12 months ago

I also find that I have to run the tests with pytest test/ from the root directory. If I do pytest . from the test directory, all the paths in the files fail to be resolved.

a-renzini commented 10 months ago

I don't quite see why a user of the package should run the tests, they are only meant for developers; that's why pytest is not a dependency. We can change the wording in the documentation to be clearer but I would be against adding the dependency for users (in fact I don't think the test folder is included in the standard pip release). I will post here once I edit the docs!

Sbozzolo commented 10 months ago

Indeed, it should not be a direct dependency. You could define dev dependencies (I don't know how this is done with your build system), or at least provide steps in the documentation.

Also, please, have a look at my other comment too:

I also find that I have to run the tests with pytest test/ from the root directory. If I do pytest . from the test directory, all the paths in the files fail to be resolved.

a-renzini commented 9 months ago

Indeed, it should not be a direct dependency. You could define dev dependencies (I don't know how this is done with your build system), or at least provide steps in the documentation.

I added a [dev] option that installs pytest and that can be specified during installation:

pip install -e .[dev]

I also find that I have to run the tests with pytest test/ from the root directory. If I do pytest . from the test directory, all the paths in the files fail to be resolved.

I don't see this as a big issue as you're not supposed to run tests from the test folder; anyways, I've added a path variable in conftest.py which solves the issue.

Sbozzolo commented 9 months ago

I don't see this as a big issue as you're not supposed to run tests from the test folder; anyways, I've added a path variable in conftest.py which solves the issue.

I think that I got confused when I read the documentation and saw that you have to run pytest ., I had assumed that was from tests. Thanks for fixing this!