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

create datapipe for resampled format #6

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

vrym2
Copy link
Member

@vrym2 vrym2 commented Jan 13, 2023

Pull Request

Description

Creating data pipes to for the resampled format of the Solar farm data

Fixes #

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration

  • Yes

If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?

  • 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

@vrym2 vrym2 linked an issue Jan 13, 2023 that may be closed by this pull request
@vrym2 vrym2 requested a review from jacobbieker January 13, 2023 12:44
@vrym2 vrym2 self-assigned this Jan 13, 2023
@vrym2 vrym2 added the enhancement New feature or request label Jan 13, 2023
@vrym2 vrym2 mentioned this pull request Jan 13, 2023
4 tasks
Copy link
Member

@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.

I don't see a datapipe or conversion script here for the raw data to netCDF? I see the stuff from the other PR here, but yeah, I'd recommend not requesting a review till the code is ready to go. And maybe split out the common changes between this PR and #5 into a smaller PR of its own, so the changes aren't duplicated, and its easier to tell what the changes are.

.flake8 Outdated
@@ -0,0 +1,2 @@
[flake8]
Copy link
Member

Choose a reason for hiding this comment

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

For these more util=y kinda ones, especially since they are the same between this PR and the other one, I would make them its own PR and merge that instead, so this PR is just focused on the datapipe bit, and not the extra formatting bits.

@vrym2
Copy link
Member Author

vrym2 commented Jan 13, 2023

I don't see a datapipe or conversion script here for the raw data to netCDF? I see the stuff from the other PR here, but yeah, I'd recommend not requesting a review till the code is ready to go. And maybe split out the common changes between this PR and #5 into a smaller PR of its own, so the changes aren't duplicated, and its easier to tell what the changes are.

Sure, sorry about that, I'll do that right away!

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@c5caf2d). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main       #6   +/-   ##
=======================================
  Coverage        ?   97.22%           
=======================================
  Files           ?        3           
  Lines           ?       36           
  Branches        ?        0           
=======================================
  Hits            ?       35           
  Misses          ?        1           
  Partials        ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@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 pretty good! One thing with the DataArray, just for naming the dimensions to match what we use in other NetCDF files, but yeah, looking good!

ukpn/scripts/download_data.py Show resolved Hide resolved
.gitignore Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Datapipe to load in raw data into resampled format
3 participants