Skip to content

Unit Testing for gtep_data.py#53

Open
belle-storm wants to merge 31 commits into
IDAES:mainfrom
belle-storm:unit_test_data-(#48)
Open

Unit Testing for gtep_data.py#53
belle-storm wants to merge 31 commits into
IDAES:mainfrom
belle-storm:unit_test_data-(#48)

Conversation

@belle-storm
Copy link
Copy Markdown
Collaborator

@belle-storm belle-storm commented Feb 12, 2026

Fixes #48

@belle-storm
Copy link
Copy Markdown
Collaborator Author

waiting on PR #61 to be merged.

@belle-storm belle-storm changed the title [WIP] Unit Testing for gtep_data.py Unit Testing for gtep_data.py Apr 28, 2026
@belle-storm
Copy link
Copy Markdown
Collaborator Author

ready for review

Copy link
Copy Markdown
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

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

@belle-storm the structure of this testing file seems more complex than I would have expected and doesn't match the format of the other testing files. It would be helpful if you could walk me through the logic here.

Comment thread gtep/gtep_data.py Outdated
Comment thread gtep/tests/unit/test_gtep_data.py Outdated
@blnicho blnicho added the Priority:High High Priority Issue or PR label May 18, 2026
Copy link
Copy Markdown
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

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

@belle-storm here are the rest of my review comments in addition to what we talked about today.

Comment thread gtep/gtep_data.py Outdated
Comment thread gtep/gtep_data.py
self.representative_weights = {
key: weight_per_date
for date, key in enumerate(self.representative_dates)
for key, date in enumerate(self.representative_dates)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did the order of key and date change here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In our conversation on Monday, we talked about whether the key should be the index or the date itself.
The previous version of representative_weights that was hard coded in (prior to #61) , used the index as the key to call.

Screenshot 2026-05-21 140607

I'm not sure why it was swapped throughout the iterations, but it seems like it should be swapped back.

Comment thread gtep/tests/unit/test_gtep_data.py Outdated
Comment thread gtep/tests/unit/test_gtep_data.py
Comment thread gtep/tests/unit/test_gtep_data.py
Comment thread gtep/tests/unit/test_gtep_data.py Outdated
Comment thread gtep/tests/unit/test_gtep_data.py Outdated
Comment thread gtep/tests/unit/test_gtep_data.py Outdated
Comment thread gtep/tests/unit/test_gtep_data.py
Comment thread gtep/tests/unit/test_gtep_data.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:High High Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests for gtep_data.py

2 participants