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

Incomplete requirements specification #90

Closed
pnsaevik opened this issue Oct 30, 2023 · 5 comments
Closed

Incomplete requirements specification #90

pnsaevik opened this issue Oct 30, 2023 · 5 comments

Comments

@pnsaevik
Copy link

The list of required and optional dependencies is outdated. The listed optional dependencies are already included in the pip required dependencies list, except matplotlib which is not used in the package itself as far as I can see. But the "required dependencies" list can be removed from the documentation I think, as long as the pip installation process takes care of it. If extra installation steps are required (on Windows for instance), these should be included here.

The optional dependency list should include packages that are required to run the tutorials (zarr, pooch etc.).

This issue is a part of a JOSS review at openjournals/joss-reviews#5967

@pnsaevik
Copy link
Author

Also, when listing optional dependencies, it should be clearly defined what the dependency is used for. This way, users can know whether they need to install the dependency or not.

@MaceKuailv
Copy link
Owner

Thanks for the suggestion. I have made some changes on that front.

dask became a required dependency pretty recently, so it is moved to where . We also didn't realize that pandas is a dependency of xarray, so now it is removed.

We don't have a lot of required dependencies, so instead of removing them I put them after the optional ones. Like you mentioned, they are not very important to users installing packages. However, environments and dependencies can get very nasty sometimes. I think it does not hurt to put some clue in the documentation.

I have tried installation on my windows laptop (~2 month ago) with anaconda. It does not require additional steps.

@pnsaevik
Copy link
Author

As long as your package uses pandas explicitly, it makes sense to list it as a dependency (at least in pyproject.toml). If xarray becomes independent from pandas in the future, your package will break if pandas is not a requirement. (Okay, this is likely not gonna happen. But my point is that an explicit dependency is not necessarily a bad thing. Treat pandas like you would treat numpy, which is also required by most packages).

But don't forget the missing optional dependencies. As it stands, users cannot run the tutorials unless installing the extra packages. It should be clearly stated which packages are required to run the examples.

@MaceKuailv
Copy link
Owner

Yes, that makes sense. I have just re-included pandas and added the packages needed for running the tutorial examples.

Part of the tutorials are ran on Sciserver, which is a publicly-accessible compute platform with readily available docker environments. There are a few packages like tqdm in the CI environment file that is never used in any of the non-Sciserver examples. They are also already installed on Sciserver. I have excluded them in the optional dependencies, because I think the users don't need to know about that.

@MaceKuailv
Copy link
Owner

Resolved with PR #95

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

No branches or pull requests

2 participants