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 Oct 25, 2021

Pull Request

Description

This PR is part of #213.

Specifically, this PR is about implementing compute_and_save_positions_of_each_example_of_each_split() and its helper functions.

This PR implements:

  • Removed a few unused imports.
  • A new class, DataSourceList, for working with lists of DataSources 🙂
  • NowcastingDataModule._get_t0_datetimes() is moved into DataSourceList.get_t0_datetimes_across_all_data_sources().
  • Implement DataSourceList.sample_spatial_and_temporal_locations_for_examples()
  • Rename DataSource.get_locations_for_batch() to DataSource.get_locations() (this touches a lot of files but is a simple change!)
  • Use a namedtuple as the return type from split_data (to make sure we don't confuse train, test, and validation!)
  • Removed some NotImplemented functions from child classes.
  • Test DataSourceList.sample_spatial_and_temporal_locations_for_examples()

How Has This Been Tested?

Tests have been updated and all pass.

  • 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

@JackKelly JackKelly self-assigned this Oct 25, 2021
@JackKelly JackKelly linked an issue Oct 25, 2021 that may be closed by this pull request
38 tasks
@JackKelly JackKelly marked this pull request as ready for review October 25, 2021 12:59
@JackKelly JackKelly requested a review from jacobbieker October 25, 2021 12:59
@JackKelly
Copy link
Contributor Author

Hi @jacobbieker I think this is pretty much ready for review now!

It looks like a lot of changes but the core changes are pretty simple!

I haven't yet written a test for DataSourceList.sample_position_of_every_example_of_every_split(). I will do so in the next few hours. But, apart from that, I think this PR is ready for review. Thank you!

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! I like how its done!

@JackKelly JackKelly changed the title Implement compute_and_save_positions_of_each_example_of_each_split() Implement DataSourceList.sample_spatial_and_temporal_locations_for_examples() Oct 25, 2021
@JackKelly JackKelly merged commit 3143ebb into main Oct 25, 2021
@JackKelly JackKelly deleted the jack/compute_and_save_positions_of_each_example branch October 25, 2021 17:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Big new design" for nowcasting_dataset

2 participants