Pirate-Weather / pirateweather

Code and documentation for the Pirate Weather API
Apache License 2.0
670 stars 29 forks source link

A couple of Time Machine Issues #330

Closed cloneofghosts closed 1 month ago

cloneofghosts commented 2 months ago

Describe the issue

I had posted this in #130 but I figure I'd post it here. Most of this is an issue when accessing the ERA5 dataset and not the PW archive

  1. When including a time parameter the connection is not secure
  2. windGust should be right below windSpeed
  3. precipIntensity only appears on the currently block and nowhere else
  4. The icon calculation seems to not be working correctly still
{
  "time": 1714521600,
  "icon": "cloudy",
  "summary": "Cloudy",
  "precipAccumulation": 0,
  "precipType": "rain",
  "temperature": 7.27,
  "apparentTemperature": 6,
  "dewPoint": 4.26,
  "pressure": 1014.2,
  "windSpeed": 7.21,
  "windBearing": 73.7,
  "cloudCover": 0.62,
  "snowAccumulation": 0,
  "windGust": 14.23
},
  1. Negative longitudes do not display correctly. -75 displays as 285
  2. When accessing the PW archive there are extra blocks/summaries not present when you access the ERA5 data. Minutely, alerts, flags, hourly/daily summaries probably shouldn't appear
  3. In line with above the extra fields that are shown when accessing the PW archive should probably be behind a query string? Would be better to keep things consistent between ERA5 and the PW archive.
  4. precipAccumulation is rounded to two decimal places instead of four
  5. The ERA5 data has a snowAccumulation data point which shouldn't be there

Acknowledgements

alexander0042 commented 2 months ago

Great clean-up catches here! To address in detail:

  1. Weird Cloudfront issue, seems to be fixed now.
  2. Fixed!
  3. This is intentional. ERA5 doesn't have historic instant intensity, only accumulations, so everything is per hour. For currently, the hourly average is used since current accumulation doesn't make sense, but I think it's clearer to label it as an accumulation in the other blocks.
  4. Fixed to align with forecast!
  5. Fixed!
  6. Mostly fixed! Agree with the consistency- I removed minutely and alerts, and added flags into older requests to keep it consistent, as well as removed missing fields from PW Archive requests.
  7. Agreed! Added a new query param called "tmextra" to show all the data in the PW Archive
  8. I really like trying to show snow vs rain, so since this data point is in the NCAR archive and the PW one, I figured it was worth including!
cloneofghosts commented 2 months ago
  1. This is intentional. ERA5 doesn't have historic instant intensity, only accumulations, so everything is per hour. For currently, the hourly average is used since current accumulation doesn't make sense, but I think it's clearer to label it as an accumulation in the other blocks.

Isn't precipIntensity just precipAccumulation multiplied by 10 to convert from cm to mm? You could probably just do the same here no?

  1. I really like trying to show snow vs rain, so since this data point is in the NCAR archive and the PW one, I figured it was worth including!

The only reason I mentioned it is that Dark Sky never had a snowAccumulation field in their data points.

alexander0042 commented 2 months ago

Re intensity, the answer is that it depends. For NBM, which is usually the source accumulation is 10x intensity. However, for GFS and HRRR rate and intensity are two separate products (PRATE and APCP), so can diverge. That's the rational for specifying intensity vs accumulation in time machine, but as I think about it, I sort of agree that it's easier for everyone if they're both in there

alexander0042 commented 2 months ago

Ok, added intensity back in, should propagate out in the next hour or so!

cloneofghosts commented 1 month ago

Everything is looking good for the most part but I see two minor issues:

PW archive data has visibility in the daily block when it should be behind the query string and the summaries for the hourly blocks and daily block are still there.

  hourly": {
  "summary": "Clear",
  "icon": "clear-day",
  "data": [
  {

Was this missed or will it be there for the PW Archive data? The ERA5 data doesn't have them showing so should ideally be consistent.

alexander0042 commented 1 month ago

You are amazing with these sorts of catches! It was just missed, fixing it now

cloneofghosts commented 1 month ago

Just checked and things are looking good now so can close.