-
Notifications
You must be signed in to change notification settings - Fork 107
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
Release v0.8.0 #443
Release v0.8.0 #443
Conversation
👈 Launch a binder notebook on this branch for commit 26e96e8 I will automatically update this comment whenever this PR is modified 👈 Launch a binder notebook on this branch for commit 9d58250 👈 Launch a binder notebook on this branch for commit 7196f18 👈 Launch a binder notebook on this branch for commit ecf6027 👈 Launch a binder notebook on this branch for commit 5b6eb84 👈 Launch a binder notebook on this branch for commit 2e0d045 👈 Launch a binder notebook on this branch for commit dba38b9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good. One comment - I'm realizing that by removing intake we have technically make a non-backwards compatible change, because the catalog
parameter was removed from Read
. It would have been very difficult to keep catalog
and still move forward with cloud read development, so I think it's the correct decision not to try to support it. That being said, I see the options as 1) if we think that very few users were using catalog
, we could merge the change into 0.8.0 anyway 2) back this release up 1 PR and include the "remove intake" change in the 1.0.0 release. It looks like "remove intake" was the most recent commit. Either way we could add a DepreciationError for the catalog argument, which I should have thought to do when I made that PR.
I just googled this ^(previous comment). If we were merging directly (without a PR) it seems pretty straightforward. But since we want to have a PR it gets a half step trickier. I think the options are either to:
Option 1 seems the smoothest from my bit of reading. Both are effort, though. I'm happy to help do the monkeying around, if that's what we want. @JessicaS11 I'll defer to you if you think doing this is worth it in order to not make a breaking change to |
@rwegener2 it sounds like option 3 (a quick PR to add a DeprecationError) is the cleanest approach to not have to monkey with reverting PRs. Note I just updated the release branch to merge in #445. |
#446 has been created which adds the Depreciation Error for catalog |
I just approved #446. After you merge could you add it to the release log here? I'm not sure why the Travis tests started failing, but it looks like a bunch of the viz ones again so we may just need to comment them out if it's not simple size mismatch updates. They seem to go through phases of working... |
merged and added as a release ✅ The fact that the viz tests seem to ephemerally pass/fail is frustrating. Commenting them seems like a good idea for now, and also creating a ticket to document anything we know about the issue so it can be revisited in depth at another time. |
Good call - we've definitely had a few issues in this vein (I think I just finally got them all closed this spring!). If you're able to merge this today and then open a PR for merging dev into main, I can approve and tag the release so it can get through conda-forge before I go completely offline later today/tomorrow am. |
Setting up for v0.8.0. We've got a bunch of new features already integrated, and a few more in the pipeline. Happy for suggestions on whether we should wait for more of the new things or plan on a few releases in the next couple months.
EDIT: we're going to release v0.8.0, with the planned release of v1.0.0 this fall once cloud read and argo data access via QUEST are implemented. If you're interested in helping prepare a release statement and/or advertising v1.0.0, we'd welcome your involvement!