Pirate-Weather / pirateweather

Code and documentation for the Pirate Weather API
Apache License 2.0
617 stars 27 forks source link

Icons sometimes returning "none" #236

Closed trevorturk closed 3 weeks ago

trevorturk commented 1 month ago

Describe the bug

Not a major issue, but I thought I'd point it out -- for the last few weeks, the API has been returning "none" for the "icon" field in a smattering of cases. I'm not sure what the root cause might be, or what a reasonable fix might be, sorry! In my app, I generally default to "partly-cloudy" if I get a strange value, but not for any particular reason...!

Expected behavior

Any "icon" fields should conform to the Dark Sky icon list.

Actual behavior

Sometimes I get "none" as a value (not null, but the string value "none")

API Endpoint

Production

Location

Milwaukee, WI, USA

Other details

No response

Troubleshooting steps

cloneofghosts commented 1 month ago

When I was testing the 2.0 beta I used to come across the "none" icon fairly regularly but once the stable version was released I've only ran across it in one instance. I will ping @alexander0042 to look into this to see what might be going on.

I've also added the documentation tag since "none" seems to be a valid icon and is not in the list. I think the "none" icon is returned when an icon cannot be determined but not 100% if that is correct.

cloneofghosts commented 1 month ago

@alexander0042 I actually found a location which is returning "none" thanks to Merry Sky which defaulted me to a location which is having the issue. The co-ordinates are -33.492807422505265,143.2100894390616 and the issue is present in the second day of the daily forecasts

{
  "time": 1716991200,
  "icon": "none",
  "summary": "None",
  "sunriseTime": 1717017712,
  "sunsetTime": 1717054045,
  "moonPhase": 0.74,
  "precipIntensity": 0.7446,
  "precipIntensityMax": 3.5664,
  "precipIntensityMaxTime": 1717023600,
  "precipProbability": 1,
  "precipAccumulation": 2.9189,
  "precipType": "none",
  "temperatureHigh": 24.43,
  "temperatureHighTime": 1717041600,
  "temperatureLow": 13.2,
  "temperatureLowTime": 1717095600,
  "apparentTemperatureHigh": 24.39,
  "apparentTemperatureHighTime": 1717041600,
  "apparentTemperatureLow": 14.71,
  "apparentTemperatureLowTime": 1716998400,
  "dewPoint": 9.7,
  "humidity": 0.64,
  "pressure": 1006.76,
  "windSpeed": 31.42,
  "windGust": 55.1,
  "windGustTime": 1717020000,
  "windBearing": 159.7,
  "cloudCover": 0.69,
  "uvIndex": 3.36,
  "uvIndexTime": 1717038000,
  "visibility": 13.75,
  "temperatureMin": 14.75,
  "temperatureMinTime": 1716994800,
  "temperatureMax": 24.43,
  "temperatureMaxTime": 1717041600,
  "apparentTemperatureMin": 14.75,
  "apparentTemperatureMinTime": 1716994800,
  "apparentTemperatureMax": 24.39,
  "apparentTemperatureMaxTime": 1717041600
},

I think what might be happening is there is enough precipitation to show the precipitation icon but the precipType is "none" so it shows the "none" icon and text?

trevorturk commented 1 month ago

I like your precipType theory!

FWIW I don't believe the OG DS API included none as a valid icon, but I think none was allowed for precipType.

cloneofghosts commented 1 month ago

I was able to find this information on an archive version of the DarkSky docs:

icon optional A machine-readable text summary of this data point, suitable for selecting an icon for display. If defined, this property will have one of the following values: clear-day, clear-night, rain, snow, sleet, wind, fog, cloudy, partly-cloudy-day, or partly-cloudy-night. (Developers should ensure that a sensible default is defined, as additional values, such as hail, thunderstorm, or tornado, may be defined in the future.)

precipType optional The type of precipitation occurring at the given time. If defined, this property will have one of the following values: "rain", "snow", or "sleet" (which refers to each of freezing rain, ice pellets, and “wintery mix”). (If precipIntensity is zero, then this property will not be defined. Additionally, due to the lack of data in our sources, historical precipType information is usually estimated, rather than observed.)

It appears that if there is no precipitation then precipType won't be defined? I'm not super familiar with how it was handled on the forecast API to say whether this is true or not.

trevorturk commented 1 month ago

In practice I saw them returning "none" as opposed to omitting the attribute. (I don't think they omitted any attributes from my experience.)

cloneofghosts commented 1 month ago

They probably just omitted it on the Time Machine API and returned "none" in the forecast API.

The precipType probably needs to be calculated in situations where precipitation is greater than zero and precipType is none. Daily it would make sense to just use the most common precipitation type or the worst precipType if there are more than one with the same number of hours. For the hourly and currently blocks the solution would probably be to calculate the precipType based on the air temperature.

The only issue with using the air temperature to calculate the precipType is when it gets close to the freezing mark. The precipType at that point could be rain, snow or sleet but the precipType returning "none" should be pretty rare that it might not matter if we calculate it for those blocks.

alexander0042 commented 1 month ago

Thanks for bringing this to my attention, totally missed it after my first go around! I rewrote the logic for the icons in V2, which is how things like this got in. It's more realistic across the board; however, definitely the occasional edge case.

Your precipType theory is correct here, although only half the story. In short, the daily icon is calculated using a series of if statements , and in this case it should be showing the precipitation type. What's strange is that there should always be a type, so something in that logic must be off.

My hunch is this occurred on a day that was mostly no precipitation, but then some sudden rainstorm, along the lines of what @cloneofghosts was getting at. That would make the most common (which is how the icon is picked) hourly type none, but trigger the precipitation thresholds. The hourly precipitation type is derived from a raw model output, providing a percentage chance of rain, snow, sleet, or hail for each hour, and I use the largest percentage for the hour. It wouldn't have shown up in V1 since I used a much simpler temperature approach, but this allows for adding in thunderstorms or hail in the future. I should update the docs to clarify this, since it's pretty vague at the moment!

Should have time to roll a fix out tomorrow. My plan is to change how the precipType daily calculation to be the most common type, other than none, which should avoid this! In the meantime, partly cloudy is a pretty decent guess, so happy you went with that!

cloneofghosts commented 1 month ago

@alexander0042 This is what MerrySky is showing for the location which is showing none for the daily icon image

What's odd is the first day shows the none icon but the second day is showing rain.

EDIT: I checked again this morning and the location that I had posted earlier is still having the "none" icon issue so you can use that for testing.

alexander0042 commented 4 weeks ago

Thanks again for alerting me to this! Fixed it by using the most common "not none" precipitation type for the daily forecast, instead of just the most common. If all 24 hours are none, it'll still be picked as the precipitation type; however, shouldn't show up s the icon in that case. Pushed 2.0.9 to prod, so should propagate through in the next 15 minutes or so

trevorturk commented 4 weeks ago

Excellent thank you! I'll remove my workaround and let you know if I spot it again.

trevorturk commented 3 weeks ago

I haven't seen this recurring, so I think it's safe to close. Leaving open in case it's useful for doc updates if needed, tho?

cloneofghosts commented 3 weeks ago

I have a PR open for updating the docs so we can close this issue.