Skip to content

Handle pooch version 1.5.0#5529

Merged
hmaarrfk merged 4 commits intoscikit-image:mainfrom
manthey:handle-pooch-versions
Aug 23, 2021
Merged

Handle pooch version 1.5.0#5529
hmaarrfk merged 4 commits intoscikit-image:mainfrom
manthey:handle-pooch-versions

Conversation

@manthey
Copy link
Contributor

@manthey manthey commented Aug 23, 2021

This is compatible with pooch < 1.5.0, too. If such older compatibility
is not desired, the dependency version could be changed and just the new
code could be kept.

Description

Closes #5528.

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

manthey and others added 3 commits August 23, 2021 08:50
@manthey
Handle pooch version 1.5.0
3bfd4ca 
This is compatible with pooch < 1.5.0, too.  If such older compatibility
is not desired, the dependency version could be changed and just the new
code could be kept.
@hmaarrfk
Flip the order in which we try things.
0647c9c
@hmaarrfk
undo typo i introduced
c3d0055
hmaarrfk
hmaarrfk approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@hmaarrfk hmaarrfk left a comment

Choose a reason for hiding this comment

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

Thanks! I flipped the order since I typically like to try "the new standard" instead of first trying the "old" then "falling back on the new".

@hmaarrfk
Copy link
Member

hmaarrfk commented Aug 23, 2021

Thank you so much for being quick on this!

grlee77
grlee77 suggested changes Aug 23, 2021
View reviewed changes
@hmaarrfk @grlee77
Update skimage/data/__init__.py
06cfbb8 
Co-authored-by: Gregory R. Lee <grlee77@gmail.com>
@grlee77
Copy link
Contributor

grlee77 commented Aug 23, 2021

I think pooch only considers the functions/classes in the top-level pooch namespace to be the public API. The PR moving the file_hash function refers to hashes as "a new private module". I think we mistakenly assumed the submodules were public API since the names didn't start with an underscore, but apparently that isn't the case

@hmaarrfk
Copy link
Member

hmaarrfk commented Aug 23, 2021

@grlee77 how do you want to proceed to get this released? This is rather breaking for our users.

I could patch the conda-forge version, (and I might for my own channel)

@grlee77
Copy link
Contributor

grlee77 commented Aug 23, 2021

@grlee77 how do you want to proceed to get this released?

I think it is probably best to make a quick 0.18.3 release with this fix. Hopefully it should be easy using the new, automated wheel building.

The 0.19 milestone items remaining are mostly just needing reviews on some recent small bug fix PRs and one larger float32 support PR. Hopefully we can complete those soon, but I don't think in the next day or two is likely.

@hmaarrfk
Copy link
Member

hmaarrfk commented Aug 23, 2021

ok. i'll hold off on making a patched version for conda-forge unless an other user complains before we have to release.

I'm not sure I can spend daylight hours pushing a sanctionned pip fix through.

@hmaarrfk hmaarrfk enabled auto-merge (squash) August 23, 2021 14:07
@hakonanes hakonanes mentioned this pull request Aug 23, 2021
Closed
@hmaarrfk hmaarrfk merged commit e263200 into scikit-image:main Aug 23, 2021
@grlee77
Copy link
Contributor

grlee77 commented Aug 23, 2021

I'm not sure I can spend daylight hours pushing a sanctioned pip fix through.

I posted here about potentially making a 0.18.3 release: https://skimage.zulipchat.com/#narrow/stream/181448-development/topic/v0.2E18.2E3.20release.3F

@leouieda leouieda mentioned this pull request Aug 23, 2021
Closed
@grlee77 grlee77 mentioned this pull request Aug 23, 2021
Merged
11 tasks
@grlee77
Copy link
Contributor

grlee77 commented Aug 23, 2021

@meeseeksdev backport to v0.18.x

@lumberbot-app
Copy link

lumberbot-app bot commented Aug 23, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v0.18.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 e263200bfa19af2b4c7dd357e9de6dc559e83c27
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #5529: Handle pooch version 1.5.0'
  1. Push to a named branch :
git push YOURFORK v0.18.x:auto-backport-of-pr-5529-on-v0.18.x
  1. Create a PR against branch v0.18.x, I would have named this PR:

"Backport PR #5529 on branch v0.18.x (Handle pooch version 1.5.0)"

And apply the correct labels and milestones.

Congratulation you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove Still Needs Manual Backport label once the PR gets merged.

If these instruction are inaccurate, feel free to suggest an improvement.

@lumberbot-app lumberbot-app bot added the Still Needs Manual Backport MrMeeseeks-managed label label Aug 23, 2021
grlee77 added a commit to grlee77/scikit-image that referenced this pull request Aug 23, 2021
@manthey @hmaarrfk @grlee77
Handle pooch version 1.5.0 (scikit-image#5529)
10a9be9 
* Handle pooch version 1.5.0

This is compatible with pooch < 1.5.0, too.  If such older compatibility
is not desired, the dependency version could be changed and just the new
code could be kept.

* Flip the order in which we try things.

* undo typo i introduced

* Update skimage/data/__init__.py

Co-authored-by: Gregory R. Lee <grlee77@gmail.com>

Co-authored-by: Mark Harfouche <mark.harfouche@gmail.com>
Co-authored-by: Gregory R. Lee <grlee77@gmail.com>
@grlee77 grlee77 mentioned this pull request Aug 23, 2021
Merged
@grlee77 grlee77 added 🩹 type: Bug fix Fixes unexpected or incorrect behavior and removed Still Needs Manual Backport MrMeeseeks-managed label labels Aug 23, 2021
@jjnesbitt jjnesbitt mentioned this pull request Aug 23, 2021
Merged
grlee77 added a commit that referenced this pull request Aug 23, 2021
@grlee77 @hmaarrfk @manthey
Backports for 0.18.3 (pooch 1.5 fix, deprecated scipy.linalg.pinv2 …
7b721a7 
…fix) (#5531)

* Backport gh-5529: Handle pooch version 1.5.0

import file_hash from Pooch public API

* Backport gh-5437: Avoid use of deprecated scipy.linalg.pinv2

pyamg used coarse_solver='pinv2' by default, but SciPy 1.7.0 has deprecated pinv2

* update version string

* DOC: add release notes for 0.18.3

Co-authored-by: Gregory R. Lee <grlee77@gmail.com>
Co-authored-by: Mark Harfouche <mark.harfouche@gmail.com>
Co-authored-by: David Manthey <david.manthey@kitware.com>
@leouieda leouieda mentioned this pull request Aug 24, 2021
Closed
@mathbunnyru mathbunnyru mentioned this pull request Aug 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hmaarrfk hmaarrfk hmaarrfk approved these changes

+1 more reviewer

@grlee77 grlee77 grlee77 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🩹 type: Bug fix Fixes unexpected or incorrect behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pooch update breaks import

3 participants

@manthey @hmaarrfk @grlee77