Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Conversation

@JackKelly
Copy link
Contributor

@JackKelly JackKelly commented Nov 18, 2021

Pull Request

Description

Fixes #435

How Has This Been Tested?

  • No
  • Yes, unit tests pass
  • prepare_ml_data.py runs

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@JackKelly
Copy link
Contributor Author

There are two commits in this PR.

I think the first commit is sufficient.

But the second commit slightly simplifies the code (but isn't strictly necessary to solve this bug!)

Copy link
Contributor

@jacobbieker jacobbieker left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for fixing it!

@JackKelly
Copy link
Contributor Author

Thanks for the quick review, @jacobbieker! Just to double-check: Are you happy for the code to always use xr.open_mfdataset? As far as I can tell, open_mfdataset does "the right thing" even if zarr_path points to a single Zarr (with no wildcards) but I haven't really put it through its paces!

@jacobbieker
Copy link
Contributor

Yeah, I think it's fine!

@JackKelly JackKelly merged commit c66ceec into main Nov 18, 2021
@JackKelly JackKelly deleted the bug/435-open-sat-data branch November 18, 2021 09:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calling xr.open_dataset in open_sat_data on a path with a wildcard in it?

4 participants