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

Some examples might require additional dependencies #5026

Open
HealthyPear opened this issue Oct 17, 2024 · 4 comments
Open

Some examples might require additional dependencies #5026

HealthyPear opened this issue Oct 17, 2024 · 4 comments
Labels
UX user-experience

Comments

@HealthyPear
Copy link

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

  • Operating System: macos 14.7
  • Python Version: 3.12.7
  • yt version: 4.3.1 via conda
  • Other Libraries (if applicable): yt-idv 0.4.1 via PyPI
Copy link

welcome bot commented Oct 17, 2024

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
Copy link
Member

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:

  • clarify the error message (how ?)
  • stop requiring pandas in load_sample (I'm convinced it's doable, but I never managed to do it)
  • implement a mechanism to report all missing dependencies at once instead of letting the user discover them iteratively (which I agree is bad UX). This is a very hard problem in the general case but maybe we could do something specifically for load_sample, which is now very commonly used in documentation.

@neutrinoceros neutrinoceros added the UX user-experience label Oct 17, 2024
@HealthyPear
Copy link
Author

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

What I would do is this:

  • add an extra set of options into pyproject.toml called examples
  • add this set of optional dependencies to the next conda recipe by default
  • update the docs so that one can either install with conda or pip install yt[examples] if the examples need to be run

@neutrinoceros
Copy link
Member

neutrinoceros commented Oct 18, 2024

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 conda-forge/astropy-feedstock#137). This even lead to a (draft) PEP: astrofrog/peps#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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX user-experience
Projects
None yet
Development

No branches or pull requests

2 participants