csparpa / pyowm

A Python wrapper around the OpenWeatherMap web API
https://pyowm.readthedocs.io
MIT License
789 stars 169 forks source link

openweathermap deprecating 2.5 APIs #404

Open KeithB opened 1 year ago

KeithB commented 1 year ago

It looks like open weather map are progressing the deprecation of the 2.5 API set used by pyowm. This is the response I got from them on a support ticket about access problems with a new API key: "In general One Call 1.0 (2.5) has been replaced by 3.0. That is the reason for getting the 401 error."

3.3.0 still uses the 2.5 API set as its base URL for One Call APIs (compared to 3.0 for other APIs).

Are there any plans to adopt the 3.0 One Call APIs?

KeithB commented 1 year ago

Potential fix at https://github.com/KeithB/pyowm-onecall3/tree/dev.

Has been tested to prove connectivity with a local stub but not put through the standard pyowm test framework.

jafoobari commented 1 year ago

@KeithB I took a crack at running the pyowm test framework against the dev branch of your fork, and I'm hitting an error around one of your code changes when running the unit tests:

======================================================================
ERROR: test_one_call_from_dict_current_missing (tests.unit.weatherapi25.test_one_call.TestOneCall)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/bari/pyowm-onecall3/tests/unit/weatherapi25/test_one_call.py", line 99, in test_one_call_from_dict_current_missing
    self.assertRaises(ParseAPIResponseError, lambda: OneCall.from_dict(data))
  File "/Users/bari/.asdf/installs/python/3.10.6/lib/python3.10/unittest/case.py", line 738, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/Users/bari/.asdf/installs/python/3.10.6/lib/python3.10/unittest/case.py", line 201, in handle
    callable_obj(*args, **kwargs)
  File "/Users/bari/pyowm-onecall3/tests/unit/weatherapi25/test_one_call.py", line 99, in <lambda>
    self.assertRaises(ParseAPIResponseError, lambda: OneCall.from_dict(data))
  File "/Users/bari/pyowm-onecall3/pyowm/weatherapi25/one_call.py", line 90, in from_dict
    raise exceptions.APIResponseError("OWM API: no current or historical data returned.")
pyowm.commons.exceptions.APIResponseError: OWM API: no current or historical data returned.

The test is hitting the error here on line 17 of /tests/unit/weatherapi25/test_one_call.py.

Which is calling your code change on line 90 of pyowm/weatherapi25/one_call.py.

I tried troubleshooting it but I don't really know what's happening...I had enough trouble getting tox to play nice and run (and it's still only partially cooperating) 😅 .

I'm happy to help however I can with getting these changes fully tested and in a pull request though. Those of us using the OWM Home Assistant integration are reliant on pyowm.

KeithB commented 1 year ago

@jallen92 Thanks for that - I ran out of time today to do much more (though I'm old school and prefer dev vs test separation - well volunteered!). It's probably going to be Monday before I can take a proper look at this.

The error suggests it can't read current data from OneCall, but I have a test stub that's happy reading current data direct from OpenWeatherMap vs a test tool that's using hard coded test data (test_one_call.py).

The line in question that's failing is deliberately introducing the failure scenario as pyowm (currently) insists on current data being returned (it fails object instantiation without it) but (setting aside HA for a minute despite that being the reason I'm here!) the openweathermap API actually allows current data to be excluded from the response. Basically I took enforcing existing pyowm behaviour (mirrored in that test data from what I can tell) for stability of existing users over openweathermap optionality.

KeithB commented 1 year ago

@jallen92 What problems did you have setting tox up? I had to amend tox.ini to reflect python 3.9/3.10 but other than that, only errors I got were related to either APIs I don't pay for or non-onecall APIs still on 2.5 I don't have access to: ======================================================================== short test summary info ======================================================================== FAILED agroapi10/test_integration_polygons_api_subset.py::IntegrationTestsPolygonsAPISubset::test_polygons_CRUD - pyowm.commons.exceptions.UnauthorizedError: Invalid ... FAILED agroapi10/test_integration_soil_data.py::IntegrationTestsSoilData::test_call_soil_data - pyowm.commons.exceptions.UnauthorizedError: Invalid API Key provided FAILED airpollutionapi30/test_integration_pollutionapi30.py::IntegrationTestsPollutionAPI30::test_air_quality_at_coords - AttributeError: 'list' object has no attribu... FAILED airpollutionapi30/test_integration_pollutionapi30.py::IntegrationTestsPollutionAPI30::test_air_quality_forecast_at_coords - AssertionError: [] is not true FAILED alertapi30/test_integration_alertapi30.py::IntegrationTestsAlertAPI30::test_triggers_CRUD - pyowm.commons.exceptions.TimeoutError: API call timeouted FAILED geocodingapi10/test_integration_geocodingapi10.py::IntegrationTestsGeocodingAPI::test_reverse_geocode - AssertionError: False is not true FAILED tiles/test_integration_tile_manager.py::TesIntegrationTileManager::test_tiles_fetch - pyowm.commons.exceptions.NotFoundError: Unable to find the resource FAILED weatherapi25/test_integration.py::IntegrationTestsWebAPI25::test_forecast_at_coords_daily - pyowm.commons.exceptions.UnauthorizedError: Invalid API Key provided FAILED weatherapi25/test_integration.py::IntegrationTestsWebAPI25::test_forecast_at_id_daily - pyowm.commons.exceptions.UnauthorizedError: Invalid API Key provided ERROR agroapi10/test_integration_satellite_imagery_download.py::IntegrationTestsSatelliteImageryDownload::test_download_geotiff - pyowm.commons.exceptions.Unauthorize... ERROR agroapi10/test_integration_satellite_imagery_download.py::IntegrationTestsSatelliteImageryDownload::test_download_png - pyowm.commons.exceptions.UnauthorizedErr... ERROR agroapi10/test_integration_satellite_imagery_download.py::IntegrationTestsSatelliteImageryDownload::test_download_tile - pyowm.commons.exceptions.UnauthorizedEr... ERROR agroapi10/test_integration_satellite_imagery_download.py::IntegrationTestsSatelliteImageryDownload::test_persisting_to_disk - pyowm.commons.exceptions.Unauthori... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_detailed_search - pyowm.commons.exceptions.UnauthorizedErro... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_search_all - pyowm.commons.exceptions.UnauthorizedError: In... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_search_for_falsecolor_png_only - pyowm.commons.exceptions.U... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_search_for_geotiff_type_only - pyowm.commons.exceptions.Una... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_search_for_ndvi_preset_only - pyowm.commons.exceptions.Unau... ERROR agroapi10/test_integration_satellite_imagery_search.py::IntegrationTestsSatelliteImagerySearch::test_search_for_one_satellite - pyowm.commons.exceptions.Unautho... ERROR agroapi10/test_integration_satellite_imagery_stats.py::IntegrationTestsSatelliteImageryStats::test_stats_for_satellite_image - pyowm.commons.exceptions.Unauthor... =============================================================== 9 failed, 47 passed, 11 errors in 35.33s ================================================================

jafoobari commented 1 year ago

@KeithB the problems I ran into with setting tox up were around the specific python executable that tox was using and it not having access to a package it needed (geojson). I ended up solving it the hard way by just installing the packages in requirements.txt via the python 3.10 executable tox is using. But I could've just made my life easy and ran the tests with my own python executable and environment by running pythonX.Y setup.py test -s tests.unit manually.

And ah that makes sense regarding the errors/failures for endpoints requiring paid access. Based on what you're saying then it sounds like your changes are good to formally introduce as a pull request to the main repo!

KeithB commented 1 year ago

@jallen92 Not quite yet its not. There's a mapping issue for daily data that needs a change to avoid breaking existing users (onecall API will result in min_temp being labeled min in the object because of the way the weather parser works).

I'd also like to see the if I can get access to the 2 APIs from the weather module that failed with authorisation failures - more to find/fix anything that could break users as I've adjusted the main weather parser object.

Did you get a successful run of Tox? Always prefer a test sign off if it's possible!

jafoobari commented 1 year ago

Gotcha!

And still no luck with getting a full successful run with Tox. I may have time later today or tomorrow to try again.

KeithB commented 1 year ago

@csparpa Couple of questions on pyowm preferences/approaches - both of them basically boil down to whether this fix would be presented as "don't use if not using OneCall3" or not.

  1. The daily mapping issue I mention above seems to already be baked in to behaviours with test data showing that a temperature entry for daily will (by code) be copied to min and max values. This contradicts structures built directly in the weather.py code where the fields are named temp_min and temp_max. i.e. The weather object will have a temperature array with different definitions depending on the API call made to OWM.
  2. I'm waiting on feedback from OWM but currently assuming that old API keys with access to the 2.5 API have not all be uplifted for access to the 3.0 API.

Is the project preference here to ensure no breaking change (i.e. existing 2.5 users can take the update with no change/impact to them) or to follow OWMs lead and assume 2.5 should be discouraged/on-going use should prefer 3.0?

csparpa commented 1 year ago

@KeithB sorry for being late at the party... the approach that has been used so far for PyOWM is to shield as much as possible users and their applications from OWM quirks and changes. Therefore I would suggest that we put OneCall3 support into PyOWM 4.0 and encourage users to upgrade their PyOWM versions and adapt their clients. Unfortunately I really don't have spare time to dedicate to this - I'm in a crazily busy moment of my life. But I will eventually put effort into this.

For the moment, if you and @jallen92 have capacity, let me just set up 4.0 up as a new milestone for PyOWM

What do you think guys? And of course, anyone else interested

KeithB commented 1 year ago

@csparpa I'm happy to get the changes set and deal with wiki updates for sample usage etc. If there are any other process parts I might have missed (preferred coding styles, code review approaches etc) let me know.

Assuming a minor version change I was going to add support for OneCall3 but default to the original OneCall1 endpoint unless specifically configured. On the basis a major version change could be breaking I'll go the other way - default behaviour is OneCall3 but option for clients to configure for legacy endpoints.

Only other bit to call out is any regression testing I can do will be limited as I don't have access to any OneCall1 endpoints.

m0n1ker commented 1 year ago

Hi everyone. I found this issue from the HA issue and forum post and would be happy to help out. Aside from an unrelated bug on Windows I was able to get this project cloned and tox running.

If the plan is to do a major version bump that could introduce breaking changes, would it make sense to pull OneCall out into its own namespace and create a separate manager for it? For example instead of putting OneCall functions inside of owm.weather_manager(), should we create own.onecall_manager()?

I know the majority of data in the OneCall API fits in the existing weather classes, but if we're saying OneCall 3.0 is supported I think we should also have support for the National weather alerts it provides. It also feels like OneCall is slowly diverging from the Weather 2.5 endpoints and this would allow us to more drastically alter the parsers without impacting other APIs as they add features or modify the data structure.

KeithB commented 1 year ago

To be honest, my preference would still be a small change to the existing 3.x.x PyOWM allowing config to access OneCall3 APIs but default behaviour to OneCall1. That is fairly straight forward and gets the likes of HA working again.

I don't disagree on the potential for more change in OneCall3 (particularly given the push on their site and the other money making APIs are all V3) but without any inside information on what or when, why do a re-write/clone (comparing the OneCall API and PyOWM code it looks to be over 90% aligned) if not needed? From a library usage perspective I'd also argue that sticking with the weather manager simplifies continued use of the API and the user not having to worry about quirks/differences between Weather and OneCall APIs for now.

Happy to follow the lead of those with closer ties to PyOWM that know better than me but I'd argue for non-breaking change (i.e. config and defaults) that enables OneCall3 in PyOWM 3.x.x and kicking off 4.x.x with the introduction of new fields on top of that with the addition of national weather alerts. (I still don't think there is enough difference to warrant a onecall_manager rather than extension of weather_manager even then.) This also gets HA et al working again leaving time to debate re-structures for new features.

ncd7 commented 11 months ago

Any news on a potential fix? Thanks.

hairlesshobo commented 11 months ago

I hate to be a "me too" kinda person, but I found my way here because I am unable to use the OneCall 3.0 subscription in Home Assistant. I was wondering if there was any update here or perhaps a roadmap regarding implementing support for the v3.0 api?

csparpa commented 10 months ago

@hairlesshobo @ncd7 @KeithB unfortunately I'm unable to keep up with Pyowm 😣

Can any of you folks can take the lead on this ?

hairlesshobo commented 10 months ago

At the moment, I unfortunately do not have the spare time because I am still in the middle of a major family move, but if no one else picks this up in the next month or two, I will see if I can submit a PR to add support for the new 3.0 API.

KeithB commented 10 months ago

I should be able to find the time to prove/update the original fix I had against current OWM functionality over the next 2 to 3 weeks.

I'm happy with that and to look at other tweaks/fixes but don't expect to have the time to do major overhauls. Much as I like the idea of the bigger re-write and starting the 4.x.x branch @m0n1ker mentioned, I need to be realistic about the combination of time and my (minimal) Python abilities.

tarrant33 commented 9 months ago

I should be able to find the time to prove/update the original fix I had against current OWM functionality over the next 2 to 3 weeks.

I'm happy with that and to look at other tweaks/fixes but don't expect to have the time to do major overhauls. Much as I like the idea of the bigger re-write and starting the 4.x.x branch @m0n1ker mentioned, I need to be realistic about the combination of time and my (minimal) Python abilities.

Were you ever able to get this to work with the one call 3.0? Or find a work around?

iridris commented 3 months ago

Is this library compatible yet with 3.0? Version 2.5 is being shut down in June 2024: https://openweathermap.org/api/one-call-api