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

DEP: Loosen dependency requirements #46

Merged
merged 6 commits into from
Feb 21, 2021
Merged

DEP: Loosen dependency requirements #46

merged 6 commits into from
Feb 21, 2021

Conversation

richford
Copy link
Member

Inspired by this pyAFQ PR, this PR loosens groupyr's dependency requirements.

@coveralls
Copy link

coveralls commented Feb 20, 2021

Coverage Status

Coverage remained the same at 95.12% when pulling f328874 on dep/permissive-deps into 80c183b on main.

@richford
Copy link
Member Author

Okay, this now relaxes a lot of other dependencies and uses tox to test two different sklearn versions. It turns out our sklearn version requirements are narrow (just 0.23.1 and 0.23.2). We're limited on the lower end by using the private _validate_data method, which appeared in 0.23.1, and on the upper end by the skopt bug referenced in #47.

Other than that, I think we're good to go here.

@richford richford added the ci Improvements to continuous integration label Feb 21, 2021
@richford richford requested a review from arokem February 21, 2021 14:12
@richford richford self-assigned this Feb 21, 2021
Copy link
Member

@arokem arokem left a comment

Choose a reason for hiding this comment

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

Yep. Looks good!

@@ -173,7 +173,7 @@ def predict(self, X):
"""
scores = self.decision_function(X)
if len(scores.shape) == 1:
indices = (scores > 0).astype(np.int)
indices = (scores > 0).astype(np.int32)
Copy link
Member

Choose a reason for hiding this comment

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

Is this a Python 3.9 thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a numpy thing. I was getting warnings about np.int and np.float being deprecated in favor of either the build in Python int and float or the specific np.int32 and np.float64.

@arokem arokem merged commit d15e23b into main Feb 21, 2021
@richford richford deleted the dep/permissive-deps branch February 21, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Improvements to continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants