yt-project / yt

Main yt repository
http://yt-project.org
Other
469 stars 280 forks source link

Some examples might require additional dependencies #5026

Open HealthyPear opened 1 month ago

HealthyPear commented 1 month ago

Bug report

Bug summary

I noticed this while trying the external package yt-idv, but I thin it might be a bug here, as it seems related to the loading of sample datasets from yt itself.

It might also be a shortcoming in the documentation in this or the other project.

Feel free to move the issue there if this is not the case.

Code for reproduction

import yt

import yt_idv

ds = yt.load_sample("IsolatedGalaxy")

rc = yt_idv.render_context(height=800, width=800, gui=True)
sg = rc.add_scene(ds, "density", no_ghost=True)
rc.run()

Actual outcome

ModuleNotFoundError: No module named 'pooch'
Something went wrong while trying to lazy-import pooch. Please make sure that pooch is properly installed.
If the problem persists, please file an issue at https://github.com/yt-project/yt/issues/new

and then in series the same for pandas and h5py respectively.

Expected outcome

The example should work immediately (and it does, when those 3 dependencies are installed).

Version Information

welcome[bot] commented 1 month ago

Hi, and welcome to yt! Thanks for opening your first issue. We have an issue template that helps us to gather relevant information to help diagnosing and fixing the issue.

neutrinoceros commented 1 month ago

yes, pooch and pandas are both requires for yt.load_sample, and h5py is needed for "IsolatedGalaxy", which is an enzo data sample. Is the error message unclear ?

I can see a couple ways we could maybe improve this experience:

HealthyPear commented 1 month ago

The error message is clear enough, the problem is that it is unexpected.

What I would do is this:

neutrinoceros commented 1 month ago

It seems I already worked on this problem in the past. Here's what I found

Though I agree there's still room for improvement ! I like your solution of an adding a new set of extras. In fact, your suggestion resonates a lot with an on-going discussion that's happening over at astropy (see for instance https://github.com/conda-forge/astropy-feedstock/pull/137). This even lead to a (draft) PEP: https://github.com/astrofrog/peps/pull/1 (currently open for feedback !).

I think I would prefer naming the new extra "recommended" to mirror astropy's similar set of "optional but strongly recommended" dependencies. This would also provide a nice solution to #4690, where we couldn't agree on whether h5py should be a required dependency or not. Regarding conda, I'd tend to agree with you that it's better to include more than just hard-required dependencies to compensate for the lack of extras, as supported by pip/PyPI.

Updating the docs is an obvious yes, though we should first see if the rest of your proposal makes sense to other devs. ping @matthewturk, @chrishavlin, @cphyc, @brittonsmith, @jzuhone, @chummels, @Xarthisius