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

Implement predicted heat strain (PHS) module from ladybug-legacy code #37

Open
chriswmackey opened this issue Sep 26, 2019 · 2 comments
Labels
backlog comfort documentation For issues requiring changes to documentation enhancement New feature or request hacktoberfest help wanted Extra attention is needed ladybug

Comments

@chriswmackey
Copy link
Member

As many ladybug legacy users know, @stgeorges did a phenomenal job writing code for calculating predicted heat strain (PHS) in ladybug-leagcy. At this point, it would be great to further clean + document this code and and integrate it into ladybug-comfort such that other developers can easily calculate PHS using the ladybug-comfort package on PyPI and view documentation about the model within the online documentation for this repo.

You can see that I have already done an example of translating one of the models from @stgeorges 's components with the Heat Index (HI) in this library. This was translated over from the heat index function in the thermal indices component and the references in the docstring came from on the wikipedia page for heat index.

I am posting this issue here in the event that someone sees it and decides that they would like to take up the mantle of transferring this code, documenting it, and getting it to conform to PEP8. If anyone has interest, please feel free to post on this issue.

NOTE TO POTENTIAL CONTRIBUTORS: PHS is a fairly complex comfort model and, if this is your first contribution to ladybug_comfort, it may be better to start with a simpler model (see other issues on this repo).

@chriswmackey chriswmackey added enhancement New feature or request help wanted Extra attention is needed documentation For issues requiring changes to documentation ladybug comfort hacktoberfest labels Sep 26, 2019
@C-H-Simpson
Copy link

Hi, I came across this because I was looking for an implementation of the PHS model; I hadn't previously used ladybug. I've copied the legacy code, and made a start on making it PEP8 compliant and more readable.

I assume that you will want unit tests for the code. I could get some test values using this other software, and/or I could test against the legacy code. I don't see any tests in the legacy code.

@chriswmackey
Copy link
Member Author

@C-H-Simpson ,

I apologize for such a late response and thank you for starting to take this on. PEP8-ifying the legacy code for PHS and adding some tests is a huge help. You are right that there are no tests for the legacy code (we really didn't have good development practices in place when we first started the legacy plugin). Testing your code against some values generated from that other software would be ideal but, if this proves difficult, testing it against the legacy code is perfectly suitable for the time being.

Also, adding some well-formatted docstrings is a huge help as you can see the online docs that are generated from the code are already becoming a resource for people who are interested in the various comfort models contained in the ladybug-comfort package.

Let me know if you have any other questions and thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog comfort documentation For issues requiring changes to documentation enhancement New feature or request hacktoberfest help wanted Extra attention is needed ladybug
Projects
None yet
Development

No branches or pull requests

3 participants