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

Cut new version #30

Merged
merged 18 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
277 changes: 0 additions & 277 deletions examples/collect_hrrr_precip.py

This file was deleted.

3 changes: 1 addition & 2 deletions tests/RME_test_case.py → tests/RME/RME_test_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ class RMETestCase(unittest.TestCase):
"""
Base class for all unit tests using the RME data
"""
test_dir = Path(__file__).parent
basin_dir = test_dir.joinpath('RME')
basin_dir = Path(__file__).parent
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain moving this file? It seems to go against how SMRF/AWSM are setup to test with basins. Just trying to think of how the format fits with the other tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved this in here because I see this as the next improvement over how SMRF/AWSM does it and would also like to see this change propagate to there. If you look at the class, it basically describes the structure of the basin dir test data and is all you need in order to use the data in tests.

It also clearly separates test data from test logic, moving it in there.

Last argument I see is that the test_dir property was removed and this was never used anywhere else, showing that moving the class into the folder is the better place to keep it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does seem to be cleaner if we move that route. Then in SMRF, the RME/Lakes would be under their presepective directories.

The one major downside I see is discoverability for that class. You pretty much have to know that it's there, as in the SMRF case, it'll be buried a few directories down.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can see this downside addressed with documentation. Currently we have a need for that as a whole, on how they are set up, what parts tests what, or when it is appropriate to updated the gold files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes to documentation. But it's way easier to just code.

gold_dir = basin_dir.joinpath('gold', 'hrrr')
hrrr_dir = basin_dir.joinpath('gridded/hrrr_test')
output_path = basin_dir.joinpath('output')
Expand Down
5 changes: 5 additions & 0 deletions tests/RME/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from .RME_test_case import RMETestCase

__all__ = [
RMETestCase
]
6 changes: 5 additions & 1 deletion tests/data/hrrr/test_base_file.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import unittest

import tests.helpers
from weather_forecast_retrieval.data.hrrr.base_file import BaseFile


Expand All @@ -8,7 +9,10 @@ class TestBaseFile(unittest.TestCase):

@classmethod
def setUpClass(cls):
cls.subject = BaseFile(cls.LOGGER_NAME)
cls.subject = BaseFile(
cls.LOGGER_NAME,
config=tests.helpers.LOG_ERROR_CONFIG
)

def test_bbox_property(self):
self.assertIsNone(self.subject.bbox)
Expand Down