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

Move pyyaml to the default requirements. #577

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

turkot
Copy link
Contributor

@turkot turkot commented Dec 5, 2024

No description provided.

@turkot turkot requested a review from takluyver December 5, 2024 13:47
@turkot turkot self-assigned this Dec 5, 2024
@turkot
Copy link
Contributor Author

turkot commented Dec 5, 2024

Running this code:

from extra_data import open_run
hdf5_data = open_run(6936, 220)

with default EXtra-data installation was throwing an error:

File /gpfs/exfel/sw/software/extra-xwiz-alpha_env/lib/python3.10/site-packages/extra_data/reader.py:688, in DataCollection._load_aliases_from_file(self, aliases_path)
    685         data = json.load(f)
    687 elif aliases_path.suffix in ['.yaml', '.yml']:
--> 688     import yaml
    690     with open(aliases_path, 'r') as f:
    691         data = yaml.safe_load(f)

ModuleNotFoundError: No module named 'yaml'

Since opening run data is the base functionality of EXtra-data, I think it makes sense to move 'pyyaml' to the default requirements.

@takluyver
Copy link
Member

Just to record, the original logic of making this optional was that it's only needed if you specify a yaml file for aliases - but then we added default aliases from a yaml file in the proposal folder. The alternative would be to skip the aliases if yaml fails, but this approach seems better.

LGTM!

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

Successfully merging this pull request may close these issues.

2 participants