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

Add Local Filesystem Support #142

Merged
merged 4 commits into from
Sep 17, 2021
Merged

Add Local Filesystem Support #142

merged 4 commits into from
Sep 17, 2021

Conversation

jacobbieker
Copy link
Member

@jacobbieker jacobbieker commented Sep 17, 2021

Pull Request

Description

Allows nowcasting-dataset to read files locally, without needing to get them from GCS or AWS.

Relates to issue #121

How Has This Been Tested?

Unit test

  • No
  • 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 added enhancement New feature or request size-small data New data source or feature; or modification of existing data source infrastructure/on-premises hardware labels Sep 17, 2021
@jacobbieker jacobbieker added this to the WP1 essential tasks milestone Sep 17, 2021
@JackKelly
Copy link
Member

Nice! And good timing, as we'll need this next week to get the code running on OCF ML server :)

@jacobbieker jacobbieker self-assigned this Sep 17, 2021
@@ -159,15 +159,15 @@ def __init__(
self.gcs = None
self.s3_resource = None

assert cloud in ["gcp", "aws"]
assert cloud in ["gcp", "aws", "local"]
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should rename cloud to compute_environment or something like that? cloud = 'local' feels a bit weird. I don't have a strong opinion though!

Copy link
Member Author

Choose a reason for hiding this comment

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

If we are going for being explicit, then probably should be compute_environment, but our own stuff is still just our own local cloud too!

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, true! So, yeah, I really don't mind what we do here. Very happy to stick with cloud!

Copy link
Member

@JackKelly JackKelly 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 to me! (Although I admit I haven't yet fully wrapped my head around what's required to get nowcasting_dataset running locally!)

@jacobbieker jacobbieker merged commit 0082a8d into main Sep 17, 2021
@jacobbieker jacobbieker deleted the jacob/local branch September 17, 2021 17:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
data New data source or feature; or modification of existing data source enhancement New feature or request infrastructure/on-premises hardware size-small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants