ttytm / wthrr-the-weathercrab

🌞 🦀 🌙 Weather companion for the terminal. Rust app.
https://crates.io/crates/wthrr
MIT License
377 stars 22 forks source link

user-defined weather data sources #86

Open ttytm opened 1 year ago

ttytm commented 1 year ago

Originally posted in https://github.com/tobealive/wthrr-the-weathercrab/issues/85

kevinmatthes commented 1 year ago

I had a look on the conversation in #85 and would like to share my ideas.

In my opinion, a good starting point would be the decomposition of the hard-coded URL in Weather::get into a struct such that users are able to adjust the queried weather API by altering the URL template. Required further fields for other APIs, such as the respective keys, for example, could be of type Option such that users can omit them, if a certain API does not require them. Further fields should be added for each new supported API. The supported APIs (based on what the configuration struct allows) should be documented.

The query URL should be made into a template, in the long term. Template processors could then assemble the actual URL from the given data. In case that the provided information is insufficient, a missing API key, for instance, the current API should be the fallback solution in order to avoid the application breaking on an incorrectly configured API. The application should furthermore generate a warning message that the respective configuration is incorrect and, if possible, which fields are affected.

ttytm commented 1 year ago

Great! I guess the most challenging part will be that the data structures of the API responses differ. We will need a standardized struct which holds all the required fields for the rendered elements. As long as the necessary data is delivered by an API in one way or another, there can be a module for that API source which formats it's results into the standardized struct.

kevinmatthes commented 1 year ago

A trait might be an option, as well.

Instead of defining one monolithic super struct for all responses, there might be one struct per API response with each of these specialised structs implementing a special trait, let us name it APIResponse, for example. Processing functions and methods should accept a Box<dyn APIResponse> and rely on the interface that trait defines. Structs implementing the trait could also try to assemble missing data from the given information such that the processing of objects implementing this trait can be rather uniform.

The trait-based design would simplify the maintenance, in my opinion. If a new API is supported, it only needs to implement the already existing trait. With the monolithic super struct, major adjustments would be required if new fields become available due to the new API.

The only thing I am rather unsure about is the performance; the super struct might have the better performance for few supported APIs but I assume that the more APIs are supported, the trait approach would amortise. There should be benchmarks for both approaches in an early step. If flexibility is more important than performance -- even in the long term --, we should directly focus on the trait approach.

kevinmatthes commented 1 year ago

I am experimenting with such a trait approach for a file IO project, at the moment, thus, I mentioned the performance aspect. I made the read data implement such a trait for easier processing and uniform write-back afterwards. The key point for performance is that the read data might be either a String or a Vec<u8> and I usually require a String for processing. The output might involve another type conversion, depending of the targeted result stream.

For a text editor I previously contributed to, there was a related discussion on how to optimise the saving of the currently edited buffer if a certain post-processing shall be performed. Benchmark tests had shown that my suggested solution involving expensive type conversions between the text buffer types took about twice the time the recent procedure required.

For the IO project, I do not assume to have too large input files in the average case, so I can focus on the functionality and try to enhance the performance afterwards. Furthermore, for the API responses, most fields can be expected to not differ too much in their representation such that type conversions should not be (too) critical. But still, type conversions could be required, after all, and we need to take into account that the compiler would include further instructions to detect the actual type of the given data at runtime. Both could become a bottleneck.

ttytm commented 1 year ago

Sorry for taking so long to come back to this. I must have unknowingly marked the notification as solved as I didn't notice your last two replies.

A trait approach could be the optimal solution to this problem. Thanks for your work! If you have any new insights, please feel free to share them or submit a draft that we can work with :)

kevinmatthes commented 1 year ago

Okay, I will prepare a draft with some initial suggestions. :+1:

kevinmatthes commented 1 year ago

I am sorry, for some reasons, the forking does not work, at the moment. I will retry later.

kevinmatthes commented 1 year ago

Okay, the forking worked, so far, now.

ttytm commented 1 year ago

Hey @kevinmatthes,

Sorry for taking a few days. Thanks again for the recent PRs! I think we can continue the enhancement of the weather API in this issue 🙂.

Struct fields that are not used to display wthrr's forecasts are removed in #105. What is left in weather.rs then are fields an API should be able to provide (exceptions below) - or at least allow to calculate - if wthrr's display items should not change.

Fields in the Hourly struct which are only used to display current weather are relativehumidity, apparent_temperature, surface_pressure, dewpoint. So it would be sufficient to just get current weather data on them. Similar for measurement units, it would suffice to get them only ones instead of DailyUnits and HourlyUnits.

Currently, data keeps it's open-meteos API response structure throughout the app. It could clear things up to organize the data based on wthrrs displayed output if we add more API providers. But there would be our "super structs" I guess.

I'll check other API providers to see how things can be better documented and structured on my end and will leave an update here. Any input is welcome :+1:

ttytm commented 1 year ago

Looked into openweathermap.org/forecast5. It's available with a free api key after registration. It provides all necessary data for 5 days.

api.openweathermap.org/data/2.5/forecast?lat=44.34&lon=10.99&appid={API KEY}

JSON Response Example ```json { "cod": "200", "message": 0, "cnt": 40, "list": [ { "dt": 1661871600, "main": { "temp": 296.76, "feels_like": 296.98, "temp_min": 296.76, "temp_max": 297.87, "pressure": 1015, "sea_level": 1015, "grnd_level": 933, "humidity": 69, "temp_kf": -1.11 }, "weather": [ { "id": 500, "main": "Rain", "description": "light rain", "icon": "10d" } ], "clouds": { "all": 100 }, "wind": { "speed": 0.62, "deg": 349, "gust": 1.18 }, "visibility": 10000, "pop": 0.32, "rain": { "3h": 0.26 }, "sys": { "pod": "d" }, "dt_txt": "2022-08-30 15:00:00" }, { "dt": 1661882400, "main": { "temp": 295.45, "feels_like": 295.59, "temp_min": 292.84, "temp_max": 295.45, "pressure": 1015, "sea_level": 1015, "grnd_level": 931, "humidity": 71, "temp_kf": 2.61 }, "weather": [ { "id": 500, "main": "Rain", "description": "light rain", "icon": "10n" } ], "clouds": { "all": 96 }, "wind": { "speed": 1.97, "deg": 157, "gust": 3.39 }, "visibility": 10000, "pop": 0.33, "rain": { "3h": 0.57 }, "sys": { "pod": "n" }, "dt_txt": "2022-08-30 18:00:00" }, { "dt": 1661893200, "main": { "temp": 292.46, "feels_like": 292.54, "temp_min": 290.31, "temp_max": 292.46, "pressure": 1015, "sea_level": 1015, "grnd_level": 931, "humidity": 80, "temp_kf": 2.15 }, "weather": [ { "id": 500, "main": "Rain", "description": "light rain", "icon": "10n" } ], "clouds": { "all": 68 }, "wind": { "speed": 2.66, "deg": 210, "gust": 3.58 }, "visibility": 10000, "pop": 0.7, "rain": { "3h": 0.49 }, "sys": { "pod": "n" }, "dt_txt": "2022-08-30 21:00:00" }, .... { "dt": 1662292800, "main": { "temp": 294.93, "feels_like": 294.83, "temp_min": 294.93, "temp_max": 294.93, "pressure": 1018, "sea_level": 1018, "grnd_level": 935, "humidity": 64, "temp_kf": 0 }, "weather": [ { "id": 804, "main": "Clouds", "description": "overcast clouds", "icon": "04d" } ], "clouds": { "all": 88 }, "wind": { "speed": 1.14, "deg": 17, "gust": 1.57 }, "visibility": 10000, "pop": 0, "sys": { "pod": "d" }, "dt_txt": "2022-09-04 12:00:00" } ], "city": { "id": 3163858, "name": "Zocca", "coord": { "lat": 44.34, "lon": 10.99 }, "country": "IT", "population": 4593, "timezone": 7200, "sunrise": 1661834187, "sunset": 1661882248 } } ```

Main differences compared to open-meteo are that there is no weathercode. Instead weather object has a description and icon field. Daily summaries need to be calculated from hourly data, as far as I see from the freely available APIs.

kevinmatthes commented 1 year ago

Sorry for the late reaction, it has been a busy week.

A basic option we have is to try converting any other API's data into an Open Meteo instance using the From / TryFrom traits. That way, we can pass API response data of any API to functions and methods expecting Open Meteo data by appending a call to .into() to the concrete instances.

Due to the differences you outlined, a conversion based on the received data might be difficult, however. Do you see any options to grab the data from somewhere else, @tobealive?

kevinmatthes commented 1 year ago

Okay, grabbing the missing data from other APIs is not required in this case. In general, we should consider this possibility for further APIs where we cannot avoid doing so.

The into() approach would thus work without any problems for this one. What do you think about it, @tobealive?

ttytm commented 1 year ago

Yes, we should keep that in mind. Personally, I wouldn't take too many steps towards supporting potential cases until one of them emerges.

Very nice! There probably will be some pitfalls but I'm exited how they will be solved 😊!