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

Fix hyppo to 0.4.0 #1095

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AlonsoGuevara
Copy link

  • Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
  • Have you built the documentation (reference and/or tutorial) and verified the generated documentation is appropriate?

Reference Issues/PRs

What does this implement/fix? Briefly explain your changes.

After 0.4.0 Hyppo changed its license from MIT to PolyForm Noncommercial License 1.0.0
This PR fixes the version to the last MIT licensed release

Any other comments?

@daxpryce daxpryce requested review from daxpryce and bdpedigo February 6, 2025 19:10
@daxpryce
Copy link
Contributor

daxpryce commented Feb 6, 2025

With hyppo changing to non commercial, pulling in any version newer than 0.4.0 would be out of the question for anyone operating in a commercial space. I would assert this does not help graspologic's aims, but I do think we need to wait for @bdpedigo 's concurrence before I approve.

@bdpedigo
Copy link
Collaborator

bdpedigo commented Feb 7, 2025

I really do not want to pin to 0.4.0 in perpetuity - in fact I was about to update to the most recent version; i think we have a test failing because of an old version of hyppo.

i will check with hyppo to see why they made this change...

@bdpedigo
Copy link
Collaborator

@AlonsoGuevara @daxpryce I checked with JHU and it seems like this license change was intentional but perhaps they didn't fully understand what it meant for downstream stuff... so there's a chance it will be reversed. But regardless, I think we should assume we're stuck with this for the time being.

my only request on this PR is could you add a comment to the pyproject.toml (if that's possible) describing why we pinned hyppo?

looking forward, I expect someday pinning hyppo will get annoying as python versions/dependencies evolve. i think the future solution should be either dropping hyppo entirely or factoring the code that uses it out into a sub-project (e.g. graspologic-inference) which maybe has a more restrictive license. but i understand that that's currently a low priority item for all of us

@AlonsoGuevara
Copy link
Author

@bdpedigo Thanks for getting back on this. I'll add the note and resubmit for approval

@daxpryce
Copy link
Contributor

daxpryce commented Feb 12, 2025 via email

@AlonsoGuevara
Copy link
Author

@bdpedigo @daxpryce Updated as requested

Copy link
Contributor

@daxpryce daxpryce left a comment

Choose a reason for hiding this comment

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

lgtm

@jovo
Copy link

jovo commented Feb 13, 2025

Hi all - I'm sorry, this was my mistake. I misunderstood something communicated to me, we will update hyppo's license to be MIT, and then the plan is for it to stay there in perpetuity.

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.

4 participants