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 version limits to dependencies. #304

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

AdamHillier
Copy link
Contributor

@lgeiger I think this should prevent what happened with #303, and Dependabot will bump the version ceilings for us. But it means we'll know if new versions break things.

@AdamHillier AdamHillier requested a review from a team April 29, 2021 09:48
"numpy~=1.15",
"numpy>=1.15.0,<1.21.0",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to pin numpy here since TensorFlow tends to have a constaint on the numpy version used anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, I think we do need to declare it as a dependency because it is used in our code. Like, if somebody installed Zoo in a fresh environment, without Numpy in our deps they might see an import error for Numpy, when what they actually are missing is TF.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, I just meant we can probably keep it as numpy~=1.15 here, but I am also OK with limiting it to numpy>=1.15.0,<1.21.0 if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'd probably rather stick with this if that's okay, just for consistency if nothing else.

@AdamHillier AdamHillier merged commit a0713e5 into larq:master Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants