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

Add Canadian Meterological Center Model Support #76

Merged
merged 18 commits into from
Dec 15, 2023
Merged

Conversation

jacobbieker
Copy link
Member

@jacobbieker jacobbieker commented Dec 14, 2023

Pull Request

Description

This adds support for the GDPS model from CMC, and partly GEPS model support as well.

Relates to #12

How Has This Been Tested?

Unit tests

  • Yes

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

@jacobbieker jacobbieker self-assigned this Dec 14, 2023
@jacobbieker jacobbieker requested a review from devsjc December 15, 2023 11:06
@jacobbieker jacobbieker marked this pull request as ready for review December 15, 2023 12:58
Copy link
Collaborator

@devsjc devsjc left a comment

Choose a reason for hiding this comment

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

Done a great job of working out the order of things in the consumer Jacob! This looks grand. The only things missing for a 100% addition of a new source is the following:

  • Update main so that the new source is an option
  • "Basic" parameter set option in the client
  • Integration test on the new source using the basic parameter set
  • Passing unit test suite

Also just to check - it seems like this source doesn't need any auth, is that right?
But you've nailed the pattern, I hope it wasn't too painful to get your head around!

src/nwp_consumer/internal/inputs/cmc/client.py Outdated Show resolved Hide resolved
src/nwp_consumer/internal/inputs/cmc/client.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/__init__.py Outdated Show resolved Hide resolved
src/nwp_consumer/internal/config/env.py Outdated Show resolved Hide resolved
@jacobbieker jacobbieker requested a review from devsjc December 15, 2023 16:39
src/nwp_consumer/__init__.py Outdated Show resolved Hide resolved
@jacobbieker jacobbieker requested a review from devsjc December 15, 2023 16:46
@jacobbieker jacobbieker merged commit 8fef095 into main Dec 15, 2023
8 checks passed
@jacobbieker jacobbieker deleted the jacob/cmc branch December 15, 2023 16:49
@jacobbieker jacobbieker mentioned this pull request Dec 19, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants