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

Update NWPDataSource for new NWP Zarr #387

Merged
merged 15 commits into from
Nov 16, 2021
Merged

Update NWPDataSource for new NWP Zarr #387

merged 15 commits into from
Nov 16, 2021

Conversation

JackKelly
Copy link
Member

@JackKelly JackKelly commented Nov 15, 2021 •
edited
Loading

Pull Request

Description

Change nowcasting_dataset to use new NWP Zarr.

Fixes #371

How Has This Been Tested?

  • 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 JackKelly added the data New data source or feature; or modification of existing data source label Nov 15, 2021
@JackKelly JackKelly self-assigned this Nov 15, 2021
@JackKelly JackKelly mentioned this pull request Nov 15, 2021
Closed
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

JackKelly
JackKelly commented Nov 16, 2021
View reviewed changes
@JackKelly JackKelly marked this pull request as ready for review November 16, 2021 16:18
@JackKelly
Copy link
Member Author

JackKelly commented Nov 16, 2021 •
edited
Loading

I'm pretty sure that, once the NWP shape issue is also fixed, then we should be all set to produce NWP batches (although NWP batches will be output pretty slowly until #311 is done).

JackKelly
JackKelly commented Nov 16, 2021
View reviewed changes
peterdudfield
peterdudfield reviewed Nov 16, 2021
View reviewed changes
nowcasting_dataset/data_sources/nwp/nwp_data_source.py Outdated
# If there are any duplicated init_times then drop the duplicated init_times:
init_time = pd.DatetimeIndex(ukv["init_time"])
if not init_time.is_unique:
_LOG.warning("NWP Zarr has duplicated init_times. Fixing...")
Copy link
Contributor

Choose a reason for hiding this comment

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

good warning

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it where it says how many duplicates were dropped too. nice to have thought

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea - I'll add that! Thanks!

peterdudfield
peterdudfield approved these changes Nov 16, 2021
View reviewed changes
Copy link
Contributor

@peterdudfield peterdudfield left a comment

Choose a reason for hiding this comment

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

great work - good comments and I like the TODOs with issue numbers

peterdudfield
peterdudfield reviewed Nov 16, 2021
View reviewed changes
@JackKelly JackKelly merged commit 8be30a7 into main Nov 16, 2021
@JackKelly JackKelly deleted the jack/update-nwpdatasource branch November 16, 2021 17:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@peterdudfield peterdudfield peterdudfield approved these changes

Assignees

@JackKelly JackKelly

Labels
data New data source or feature; or modification of existing data source
Projects
No open projects
Nowcasting
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update NWPDataSource with new NWP variable names
2 participants
@JackKelly @peterdudfield