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

support pooch >= 1.7 #1727

Merged
merged 3 commits into from Jun 20, 2023
Merged

support pooch >= 1.7 #1727

merged 3 commits into from Jun 20, 2023

Conversation

bmcfee
Copy link
Member

@bmcfee bmcfee commented Jun 20, 2023

Reference Issue

Fixes #1688

What does this implement/fix? Explain your changes.

This PR rewrites the example data registry file to use the new format introduced in pooch 1.7.0.

It removes the upper pin on the pooch dependency, and bumps our minimum supported version to 1.7.

Any other comments?

Per my comments in #1688, I'm still ambivalent on this, as it would force an update to user environments that is not backward-compatible. But, since it's extremely unlikely to affect anyone, I'm leaning toward rolling it into 0.10.1 anyway.

No functionality changes here, so as long as the CI passes and builds cleanly, it should be good to merge.

@bmcfee bmcfee added the Upstream/dependency bug Another package is causing us trouble! label Jun 20, 2023
@bmcfee bmcfee added this to the 0.10.1 milestone Jun 20, 2023
@bmcfee bmcfee self-assigned this Jun 20, 2023
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (519d6d9) 98.75% compared to head (63fb438) 98.75%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1727   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files          34       34           
  Lines        4587     4587           
=======================================
  Hits         4530     4530           
  Misses         57       57           
Flag Coverage Δ
unittests 98.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@bmcfee
Copy link
Member Author

bmcfee commented Jun 20, 2023

Aha... this is going to be a pain for the historical documentation build.

I'll have to rethink this...

@bmcfee bmcfee changed the title support / require pooch >= 1.7. [WIP] support / require pooch >= 1.7. Jun 20, 2023
@bmcfee
Copy link
Member Author

bmcfee commented Jun 20, 2023

Two ideas here:

  1. (already implemented just now) is to leave the upper version pin in the docs environment. This way, old versions will continue to build with pooch 1.6, but we can still test the latest on 1.7.
  2. Instead of just reformatting the registry, we can rename the files server-side to avoid apostrophes. This can be done with a few symbolic links, and old versions will continue to work fine. Doing this will allow us to have a single registry file that works with pooch 1.6 or 1.7, so the minimum version requirement can stay where it was and we don't force environment updates on users. However, we will still need the upper version pin on the doc environment.

@bmcfee bmcfee changed the title [WIP] support / require pooch >= 1.7. support / require pooch >= 1.7. Jun 20, 2023
@bmcfee
Copy link
Member Author

bmcfee commented Jun 20, 2023

I've updated the librosa data repo to move the offending filenames, but add backward-compatible symbolic links so that old versions continue to work. I think this ought to do the trick.

@bmcfee bmcfee added the management Project governance, packaging, distribution, etc label Jun 20, 2023
@bmcfee bmcfee changed the title support / require pooch >= 1.7. support pooch >= 1.7 Jun 20, 2023
@bmcfee bmcfee merged commit f8c3b1e into main Jun 20, 2023
11 checks passed
@bmcfee bmcfee deleted the pooch17 branch June 20, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
management Project governance, packaging, distribution, etc Upstream/dependency bug Another package is causing us trouble!
Development

Successfully merging this pull request may close these issues.

Rebuild pooch registry to support 1.7.0.
1 participant