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

Replace pytz with zoneinfo. Fix for #222 #225

Merged
merged 2 commits into from
Jun 13, 2024
Merged

Conversation

dbiczo
Copy link
Contributor

@dbiczo dbiczo commented Jun 13, 2024

Fix "Detected blocking call to open inside the event loop" warnings by replacing pytz with zoneinfo.

dbiczo added 2 commits June 13, 2024 17:30
Replace pytz with zoneinfo
Replace pytz with zoneinfo
@dbiczo dbiczo marked this pull request as draft June 13, 2024 07:50
@dbiczo dbiczo marked this pull request as ready for review June 13, 2024 07:51
@Makin-Things
Copy link
Collaborator

@dbiczo thanks for this PR. For some reason I had not even been getting these warnings (probably because I Only use it for forecast information). I assume you have fully tested that it doesn't break anything else. It looks like a fairly trivial change.

@dbiczo
Copy link
Contributor Author

dbiczo commented Jun 13, 2024

Yes, tested over a full day. Havn't tested setup flow but don't think it will affect that.

@Makin-Things
Copy link
Collaborator

Awesome. Thanks I will merge and do a release.

@Makin-Things Makin-Things merged commit d1fb35f into bremor:main Jun 13, 2024
@Makin-Things
Copy link
Collaborator

Makin-Things commented Jun 13, 2024

I did the release and it broke for me so I have removed it for now. It looks like there is a typo in the fix you submitted.
File "/config/custom_components/bureau_of_meteorology/weather.py", line 8, in <module> import zoneifo ModuleNotFoundError: No module named 'zoneifo'

I will edit on main and retry.

Actually I think line 8 should simply be removed since you added the import needed on line 18. Do you agree?

@dbiczo
Copy link
Contributor Author

dbiczo commented Jun 13, 2024

Thanks. Sorry for the typo. I tested live on my setup and copied the changes to the pull request.

@dbiczo
Copy link
Contributor Author

dbiczo commented Jun 13, 2024

I was copying what was there for pytz and both lines were there.

@Makin-Things
Copy link
Collaborator

Yep, they both need to be there. I did a quick test without the first one and got errors. Will do a new release shortly.

@Makin-Things
Copy link
Collaborator

Release 1.3.2 is done and gives me no errors.

@dbiczo
Copy link
Contributor Author

dbiczo commented Jun 13, 2024

Great. Thanks for you help.

@fabienval
Copy link

That commit introduce error #226 since we are no longer using pytz, but pytz.timezone is still referenced for the UV text

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.

4 participants