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

Generalize generator type in MockProfileInput #699

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Generalize generator type in MockProfileInput #699

merged 1 commit into from
Nov 2, 2022

Conversation

rouille
Copy link
Collaborator

@rouille rouille commented Nov 2, 2022

Pull Request doc

Purpose

Generalize generator type in the MockProfileInput class by using information from the model_immutables attribute of the Grid object

What the code is doing

Replace hard coded generator type with a dictionary grouping profile to generator type that is available through an attribute of a Grid object for all grid model

Testing

Existing unit tests. To get the tests in the powersimdata.tests.test_mock module pass, I had to sort the keys in the grid.model_immutables.plants["group_profile_resources"] dictionary to mirror what was hard coded before to get the random numbers in _random assigned to the same profile.

Where to look

The powersimdata.tests.mock_profile_input module

Usage Example/Visuals

N/A

Time estimate

5min

@rouille rouille added the refactor Code that is being refactored label Nov 2, 2022
@rouille rouille self-assigned this Nov 2, 2022
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.

Ah, the random seed.

@rouille
Copy link
Collaborator Author

rouille commented Nov 2, 2022

Ah, the random seed.

It took me a good 15min to figure it out

@rouille rouille merged commit d83ac6c into develop Nov 2, 2022
@rouille rouille deleted the ben/mock branch November 2, 2022 23:26
@BainanXia
Copy link
Collaborator

Ah, the random seed.

It took me a good 15min to figure it out

Even worse for me. My brain is not in a good situation.

@jenhagg jenhagg mentioned this pull request Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code that is being refactored
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants