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

test: Use fake profile data instead of test_usa_tamu #586

Merged

Conversation

c-voegele
Copy link

Pull Request doc

Purpose

Hi Breakthrough Energy Team! Thank you for your work on this interesting and important project!

This PR is an attempt to fix #489.

The test_usa_tamu profiles are no longer needed for test_transform_profile and no Internet connection is required to run the tests.

What the code is doing

To avoid the Azure blob call, I mocked InputData to return random profiles. I just used uniform random data in [0, 1) as suggested here

Testing

I ran tox -e pytest-local -- --cov=powersimdata and tox -e checkformatting and both are passing.

To make sure that test_usa_tamu was not required, I wiped ~/ScenarioData and rebuilt it with LocalConfig().initialize(). After running the tests, test_usa_tamu is no longer loaded.

Where to look

powersimdata/input/tests/test_transform_grid.py

Usage Example/Visuals

n/a

Time estimate

15 minutes

@c-voegele c-voegele force-pushed the c-voegele/test_with_fake_data branch from d1af535 to e14f7ec Compare February 14, 2022 15:53
@c-voegele c-voegele force-pushed the c-voegele/test_with_fake_data branch from e14f7ec to 5d306f8 Compare February 15, 2022 04:17
@BainanXia
Copy link
Collaborator

This is nice! I'm wondering whether it is possible to pull mock_input_data and corresponding raw profile functions into a separate module within powersimdata/tests/ together with other mocks so that it could potentially be used elsewhere, then we could import and patch it in test_transform_profile.py.

@BainanXia
Copy link
Collaborator

@jon-hagg With the mock_input_data implemented, do we still need truncated test profiles in blob storage under test_usa_tamu folder?

@BainanXia BainanXia requested a review from kasparm February 15, 2022 20:56
@jenhagg
Copy link
Collaborator

jenhagg commented Feb 15, 2022

@jon-hagg With the mock_input_data implemented, do we still need truncated test profiles in blob storage under test_usa_tamu folder?

I would keep them for now - they are used here to test scenario preparation

@c-voegele c-voegele force-pushed the c-voegele/test_with_fake_data branch from 47e49fd to 1cc34bc Compare February 16, 2022 17:25
@c-voegele
Copy link
Author

This is nice! I'm wondering whether it is possible to pull mock_input_data and corresponding raw profile functions into a separate module within powersimdata/tests/ together with other mocks so that it could potentially be used elsewhere, then we could import and patch it in test_transform_profile.py.

Thanks for your suggestion @BainanXia! I reorganized the code as you suggested. Let me know what you think!

@c-voegele
Copy link
Author

@jon-hagg With the mock_input_data implemented, do we still need truncated test profiles in blob storage under test_usa_tamu folder?

I would keep them for now - they are used here to test scenario preparation

Perhaps I can remove this dependency on test_usa_tamu in a separate PR?

@BainanXia
Copy link
Collaborator

This is nice! I'm wondering whether it is possible to pull mock_input_data and corresponding raw profile functions into a separate module within powersimdata/tests/ together with other mocks so that it could potentially be used elsewhere, then we could import and patch it in test_transform_profile.py.

Thanks for your suggestion @BainanXia! I reorganized the code as you suggested. Let me know what you think!

Thanks for the contribution! I think we are good to go except for some minor concerns above.

@c-voegele c-voegele force-pushed the c-voegele/test_with_fake_data branch from 1cc34bc to 098d9f3 Compare February 17, 2022 16:21
@c-voegele
Copy link
Author

This is nice! I'm wondering whether it is possible to pull mock_input_data and corresponding raw profile functions into a separate module within powersimdata/tests/ together with other mocks so that it could potentially be used elsewhere, then we could import and patch it in test_transform_profile.py.

Thanks for your suggestion @BainanXia! I reorganized the code as you suggested. Let me know what you think!

Thanks for the contribution! I think we are good to go except for some minor concerns above.

Thanks! I addressed your comments. Let me know if you find anything else!

@c-voegele c-voegele requested a review from BainanXia February 17, 2022 16:23
MockInputData is a mock of powersimdata.input.input_data.InputData
that generates random profiles.

Exactly 3 of {`start_time`, `end_time`, `periods`, `freq`} must be specified. See <https://pandas.pydata.org/docs/reference/api/pandas.date_range.html>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Comment on lines +194 to +206
@pytest.fixture(scope="module")
def input_data(base_grid):
mock_input_data = MockInputData(base_grid)
return mock_input_data


@pytest.fixture(scope="module", autouse=True)
def mock_input_data_class(input_data):
with patch(
"powersimdata.input.transform_profile.InputData"
) as mock_input_data_class:
mock_input_data_class.return_value = input_data
yield
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is clean.

Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

Great! Thanks for improving test coverage and the patience @c-voegele

@c-voegele
Copy link
Author

Thanks for the approval!

It doesn't look like I have write permissions so this is as far as I can go.

@BainanXia
Copy link
Collaborator

Thanks for the approval!

It doesn't look like I have write permissions so this is as far as I can go.

No worries, we will take care of it.

@BainanXia BainanXia merged commit 733c5b1 into Breakthrough-Energy:develop Feb 18, 2022
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.

Mock profiles for testing
5 participants