openeemeter / eeweather

Fetch NCDC ISD, TMY3, or CZ2010 weather data that corresponds to ZIP Code Tabulation Areas or Latitude/Longitude.
http://eeweather.openee.io/
Apache License 2.0
50 stars 19 forks source link

`fetch_tmy3_hourly_temp_data` always returns the same csv's data #45

Closed andrew-mcharg-tendril closed 5 years ago

andrew-mcharg-tendril commented 5 years ago

fetch_tmy3_hourly_temp_data always returns the same csv's data because CSVRequestProxy caches but does not use the url as a key.

I have a fix in a branch and can open a pull request: https://github.com/andrew-mcharg-tendril/eeweather/tree/fix_csv_request_proxy

philngo commented 5 years ago

@andrew-mcharg-tendril Great! Could you open that PR?

philngo commented 5 years ago

@andrew-mcharg-tendril Yeah, this is a bug. Looking at this more closely, I'm skeptical that the CSVRequestProxy is necessary at all, and I'm leaning toward removing it entirely.

andrew-mcharg-tendril commented 5 years ago

That probably would be better I can do that. I was curious what purpose it served. Maybe for stubbing/mocking?

philngo commented 5 years ago

Yes - I think that was it. Let's for sure remove it entirely and use the higher quality mocking mechanism that you proposed in your PR. Confusing and unnecessarily messy as previously written.

philngo commented 5 years ago

Or at the very least it shouldn't be a global object.

philngo commented 5 years ago

Thanks for the bug report and fix. As you can see I went ahead and implemented the fix that removed the proxy completely, which I'm hoping will help us avoid this sort of thing in the future. The new version is available as eeweather==0.3.16.

>>> import eeweather
>>> s1 = eeweather.ISDStation('722880')
>>> s2 = eeweather.ISDStation('722860')
>>> s1.fetch_tmy3_hourly_temp_data().head()
1900-01-01 00:00:00+00:00    10.8
1900-01-01 01:00:00+00:00     9.5
1900-01-01 02:00:00+00:00     8.8
1900-01-01 03:00:00+00:00     8.3
1900-01-01 04:00:00+00:00     8.0
Freq: H, dtype: float64
>>> s2.fetch_tmy3_hourly_temp_data().head()
1900-01-01 00:00:00+00:00    12.0
1900-01-01 01:00:00+00:00     9.0
1900-01-01 02:00:00+00:00     8.0
1900-01-01 03:00:00+00:00     8.0
1900-01-01 04:00:00+00:00     8.4
Freq: H, dtype: float64
andrew-mcharg-tendril commented 5 years ago

Thanks!