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

Testcases for template #171

Merged
merged 3 commits into from
May 27, 2024
Merged

Testcases for template #171

merged 3 commits into from
May 27, 2024

Conversation

dsteinkopf
Copy link
Collaborator

add tests for template and add docu for path in json

@dsteinkopf dsteinkopf requested review from 0x7878 and henne49 May 10, 2024 10:07
Copy link
Collaborator

@0x7878 0x7878 left a comment

Choose a reason for hiding this comment

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

Cool that you wrote a test for it too. I assume it works as intended. It is your decision whether or not to include the type in the review.

@@ -171,10 +171,10 @@ def _get_config():
return config

@staticmethod
def get_processed_meter_value(meter_data: dict, value: str, default_value: any, factor: int = 1) -> any:
def get_processed_meter_value(meter_data: dict, path_to_value, default_value: any, factor: int = 1) -> any:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove the type?

Suggested change
def get_processed_meter_value(meter_data: dict, path_to_value, default_value: any, factor: int = 1) -> any:
def get_processed_meter_value(meter_data: dict, path_to_value: str, default_value: any, factor: int = 1) -> any:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because this is actually not a string - it is an array of strings. But not always... So the "str" here is more confusing than helping

We should clean this up...

@@ -484,7 +484,7 @@ def fetch_url(self, url, try_number=1):
else:
raise

def _get_data(self):
def _get_data(self) -> dict:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice!

@dsteinkopf dsteinkopf merged commit fdf1040 into main May 27, 2024
6 checks passed
@dsteinkopf dsteinkopf deleted the testcases_for_template branch May 27, 2024 09:55
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.

2 participants