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

Add rdata as a test dependency #262

Open
nathanjmcdougall opened this issue Jul 20, 2024 · 2 comments · May be fixed by #265
Open

Add rdata as a test dependency #262

nathanjmcdougall opened this issue Jul 20, 2024 · 2 comments · May be fixed by #265
Labels
.maint upkeep and maintenance of the pkg testing

Comments

@nathanjmcdougall
Copy link
Contributor

At the moment, rdata is similar to pyarrow, etc. in the sense that it is optional, but still needed for testing and so I think it should be explicitly declared as a optional test dependency in setup.cfg.

This will mean it can be added as a pinned version to requirements/dev.txt rather than installed separately. This ensures consistency between local dev and CI and also ensures there is a consistent dependency resolution with dev.txt together with the rdata package.

I would like to do this, then the separate install step could be removed:

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -r requirements/dev.txt
python -m pip install rdata
python -m pip install -e .[test]

Incidentally, I am not sure why python -m pip install -e .[test] is necessary since the test extra is already included in the dev.txt file. I think this could be removed too.

@nathanjmcdougall
Copy link
Contributor Author

This is also the only thing keeping us from using reportMissingImports = true in Pyright (see #256).

@isabelizimm
Copy link
Collaborator

isabelizimm commented Jul 22, 2024

Thanks for this bit of cleanup, these seem like great ideas to do a bit of maintenance! I would want to make sure that there is at least one set of tests run that does not include all the test dependencies, but that case should be covered by the min test requirements CI run.

since the test extra is already included in the dev.txt file.

You are correct; it should be python -m pip install -e . 👀 pyarrow and fastparquet were not part of dev.txt when that line was added, but we have since updated that requirements file to be comprehensive.

@isabelizimm isabelizimm added testing .maint upkeep and maintenance of the pkg labels Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.maint upkeep and maintenance of the pkg testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants