feat: legacy effective temperature #379
Conversation
|
@chriswmackey the original had a bunch of args that were not used. |
There was a problem hiding this comment.
Thanks, @TfedUD ,
You were right to strip out the unused inputs. There are just a few more tweaks to be made before merging. Notably, the code should be following PEP8 in the capitalization of the variables and the spacing around operators. I recommend pip installing flake8 or some other liter to help make sure your code is following PEP8.
ladybug_comfort/et.py
Outdated
| # -*- coding: utf-8 -*- | ||
| """Effective temperature transitioned from legacy code of @stgeorges""" | ||
|
|
||
| def effective_temperature(Ta, ws, rh, SR, ac): |
ladybug_comfort/et.py
Outdated
|
|
||
| if TRE < 1: | ||
| effectTE = -4 | ||
| comfortable = 0 |
There was a problem hiding this comment.
This whole set of conditionals should be in a separate function. Much like you see here in the apparent temperature module:
https://github.com/ladybug-tools/ladybug-comfort/blob/master/ladybug_comfort/at.py
Also, we don't need the 0/1 for comfortable. We can infer that from the category.
There was a problem hiding this comment.
@chriswmackey im working on documentation at the moment and had a question, do you know what the variable effectTE is? I would like to document it
ladybug_comfort/et.py
Outdated
| TE = Ta - 0.4*(Ta - 10)*(1-rh/100) | ||
| elif ws > 0.2: | ||
| # modified formula by Gregorczuk (WMO, 1972; Hentschel, 1987) | ||
| TE = 37 - ( (37-Ta)/(0.68-(0.0014*rh)+(1/(1.76+1.4*(ws**0.75)))) ) - (0.29 * Ta * (1-0.01*rh)) |
There was a problem hiding this comment.
The spacing here needs to follow PEP8. As do all of the variable names:
|
Sounds good @chriswmackey I should be able to make the changes later this week if not next! |
|
For some reason I couldn't get a linter to fix on save (problem for another day) but it turned into a great learning experience looking up the needed rules and reading through the docs on PEP8. fun file to work on |
| * tre: Radiant Effective Temperature | ||
| * effectTE | ||
| * Comfort: boolean value for whether or not the effective temp is comfortable | ||
| """ |
There was a problem hiding this comment.
This is very close to being merge-able. There's just one last change that we need to make. These docstrings are not formatted properly to be picked up by our Sphinx workflow that generates the Ladybug Tools SDK Doc pages. Once they are formatted similarly to the apparent temperature module, we will be in good shape to merge.
| # Radiative-effective temperature | ||
| tre = te + ((1 - 0.01 * ac) * sr) * ((0.0155 - 0.00025 * te) - (0.0043 - 0.00011 * te)) | ||
|
|
||
| return effective_temperature_conditions(tre, te) |
There was a problem hiding this comment.
Actually, I lied. This function should only return the TE and the TRE. No boolean value (is the docstring wrong?) And no category. the categorization is done separately.
There was a problem hiding this comment.
you are correct! That was a mistake in the docstring
| * ta: Air Temperature | ||
| * ws: Wind Speed | ||
| * rh: Relative Humidity | ||
| * sr: Solar Radiation |
There was a problem hiding this comment.
Also, what are the units of solar radiation here? And the units of all of these inputs for that matter?
| * te: Thermal emission | ||
| Outputs: | ||
| * tre: Radiant effective temperature | ||
| * te: Effective temperature |
There was a problem hiding this comment.
Also this does not look right from what is returned.
This PR adds the effective code for effective temperature from stgeorges code.