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 numpy dependency #52

Merged
merged 2 commits into from
Mar 13, 2023
Merged

Add numpy dependency #52

merged 2 commits into from
Mar 13, 2023

Conversation

noelia-mari
Copy link
Contributor

@noelia-mari noelia-mari commented Mar 13, 2023

As storage/files.py is importing pandas and it needs the numpy dependency xocto should have it as an extra required dependency.

This commit adds the numpy dependency used by pandas (imported in storage/files.py).
@noelia-mari noelia-mari marked this pull request as ready for review March 13, 2023 08:06
@noelia-mari noelia-mari requested review from pydanny and cgl and removed request for pydanny March 13, 2023 08:06
Copy link
Contributor

@pydanny pydanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! 🙂

I'm not certain this is necessary and a pinned numpy version might cause conflict downstream. Reading this from my phone, Pandas has numpy defined as a dependency across a range of versions.

Of course I might be wrong or you might be looking at something I'm not seeing.

Copy link
Contributor

@pydanny pydanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after talking to @noelia-mari, pandas isn't installing numpy by default.

Separately, I'll raise an issue about having xocto dependencies being a range rather than pinned dependencies.

@noelia-mari noelia-mari merged commit e87ca8e into main Mar 13, 2023
@noelia-mari noelia-mari deleted the add-dependency branch March 13, 2023 09:49
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