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

Ported FMI "feels like" temperature calculation. #52

Merged
merged 3 commits into from Feb 27, 2024

Conversation

jaenis
Copy link
Contributor

@jaenis jaenis commented Feb 12, 2024

Added temperature "feels like" calculation based on FMI's original C code.
Bumped version to 0.2.1

Copy link
Owner

@saaste saaste left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution and sorry for the delay! I was on vacation so took a while to check GitHub ☺️

It looks like the older version of pylint has a bug that makes it hate short variable names. And to be honest, I'm not a big fan of them either, except in loops 😄

The newer pylint fixes the issue, but it does not support Python 3.7. I could drop the support of 3.7 a bit later since it is already in end-of-life state, but for the time being I'd appreciate if you could change the code so that it makes the older pylint happy ☺️

fmi_weather_client/parsers/forecast.py Outdated Show resolved Hide resolved
fmi_weather_client/parsers/forecast.py Outdated Show resolved Hide resolved
@saaste
Copy link
Owner

saaste commented Feb 18, 2024

One more request: could you please update README.md? "Feels like" is such a useful field it could be mentioned under "FMI provides the following commonly used information" ☺️

@jaenis
Copy link
Contributor Author

jaenis commented Feb 25, 2024

Did updates. Should they be squashed into one commit - or kept as separate? Really up to you 😃

@saaste
Copy link
Owner

saaste commented Feb 27, 2024

Awesome! Looks good! I'll merge this and release a new version after my workday ☺️

Thanks again for the contribution! 🙏

@jaenis
Copy link
Contributor Author

jaenis commented Feb 27, 2024

Thanks!

@saaste saaste merged commit 7e574e0 into saaste:master Feb 27, 2024
5 checks passed
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.

None yet

2 participants