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

Refactor of the big if-chain to a dictionary in the form {backend_name: backend_open}. #4431

Merged
merged 3 commits into from
Sep 24, 2020
Merged

Refactor of the big if-chain to a dictionary in the form {backend_name: backend_open}. #4431

merged 3 commits into from
Sep 24, 2020

Conversation

alexamici
Copy link
Collaborator

@alexamici alexamici commented Sep 17, 2020

This ports the clean up of the messy if-else-chain from #3166. It should make adding the new back-end API easier.

The change is relatively trivial, but it touches code in xr.open_dataset so extra care should be used by reviewers. We think the change is safe and it does pass all tests.

I cannot add reviewers myself, but we agreed this should go to @jhamman.

@alexamici alexamici marked this pull request as draft September 17, 2020 18:15
@alexamici
Copy link
Collaborator Author

alexamici commented Sep 17, 2020

@jhamman: @shoyer requested to keep this PR on hold until #4187 is merged as it will produce conflicts.

So I converted the PR to draft, after #4187 gets in, we will fix the conflicts and resubmit.

@jhamman
Copy link
Member

jhamman commented Sep 17, 2020

Thanks for the ping @alexamici - this looks good to me. Happy to see this merged once we sort out the conflicts in #4187.

@shoyer
Copy link
Member

shoyer commented Sep 22, 2020

#4187 has been merged, so this can go ahead again!

@alexamici alexamici closed this Sep 22, 2020
@alexamici
Copy link
Collaborator Author

Woops I didn't know that force pushing a branch would close the PR :/

I'll try to reopen this PR or create a new one. Sorry for the noise.

@dopplershift
Copy link
Contributor

@alexamici Force-pushing doesn't normally close it, so this is weird...

@keewis
Copy link
Collaborator

keewis commented Sep 22, 2020

the reason the PR was closed is that you removed the changes, so the diff to master is empty. I think you can reopen after pushing commits with changes.

@alexamici
Copy link
Collaborator Author

Thanks @keewis! So it is correct, I wanted to reset to master and re apply the changes.

max-sixty and others added 2 commits September 22, 2020 14:06
* Add docs re stable branch

* Update HOW_TO_RELEASE.md

Co-authored-by: keewis <keewis@users.noreply.github.com>

Co-authored-by: keewis <keewis@users.noreply.github.com>
@alexamici alexamici reopened this Sep 23, 2020
@alexamici alexamici marked this pull request as ready for review September 23, 2020 19:21
@alexamici
Copy link
Collaborator Author

@jhamman I don't understand why 4f414f2 is shown here, it is a commit in master.

As for the rest, we re applied the engine selection refactor and added the new zarr bits.

All looks reasonable to me and all tests pass.

@shoyer
Copy link
Member

shoyer commented Sep 23, 2020

@jhamman I don't understand why 4f414f2 is shown here, it is a commit in master.

I usually find it easiest just to use "merge master" to integrate the latest changes, and to treat pull requests as "append only". History gets consolidated/rewritten when we merge the PR with "Squash and Merge"

xarray/backends/api.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants