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

Documentation: rest of the dostrings heatpump rest #57

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

jgrguric96
Copy link
Collaborator

The rest of the models' documentation

@@ -274,7 +274,7 @@ def get_data(self, outputs:dict) -> dict: # to be implemented
'time': output_time (for event-based sims, optional)
}

See Also
See Alsoe
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix these once all the PRs are merged.

@jgrguric96 jgrguric96 added the documentation Improvements or additions to documentation label Oct 23, 2024
Copy link
Member

@manuGil manuGil left a comment

Choose a reason for hiding this comment

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

I left a few comments, but no further action is required. This can be merged.

If two numbers are equally close, return the smallest number.
[Josip] Since this is just fetching an item from a sorted list, this could be replaced with a binary search algorithm
Copy link
Member

Choose a reason for hiding this comment

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

It will be best to document comment/suggestions like this as issues in the repo and add a link to the code lines. That will make this more visible and likely to be picked up and fixed.

Comment on lines +1012 to +1018
Returns the minimum of the `self.P_th_stages` attribute

Returns
-------
self.P_th_stages : ???
???
"""
Copy link
Member

@manuGil manuGil Oct 24, 2024

Choose a reason for hiding this comment

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

We should recommend that missing details like in this case should be fixed when the models are translated to the new interface.

@@ -397,27 +490,65 @@ def get_nested_attr(self, nested_attr):
return getattr(if_type[name], attr)
Copy link
Member

Choose a reason for hiding this comment

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

For future reference. Exceptions are usually documented in doc strings under the heading Raises, with the format <errorType>: <description of the error/exception>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants