-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make API requests and response typesafe #4
Comments
I agree, all the advantages TS offers are not taken advantage of here thus far. Love your PoC @atombrenner ! |
Hi @atombrenner! Your library is certainly a nicer approach. I have no illusions that the provided SDK compiled FlatBuffer files and simple fetch library are fairly basic and a higher level abstraction can provide a better developer experience. The FlatBuffers integration is barely 3 months old. I first wanted to observe the adaption and potential issues. Arguably FlatBuffers is overkill for a simple weather API, but with up to 80 years of historical data, it makes a difference in parsing time. One issue for example is the FlatBuffers structure is hard to understand. I like the conversion step to One issue is the extendability with more variables. Because the API supports a never ending number of weather variables, it is hard to hardcode a list of all acceptable variables for each endpoint. With pressure level weather variables like The FlatBuffers schema encodes this information as attributes. It might be a good idea to have a |
@patrick-zippenfenig thanks for the kind response :-) I have a question to the ever-growing number of parameters. To be useful, each parameter must be documented. If new parameters are added we would just need a new version of the packages. As you mentioned, the schema of the parameters seems to be optimizable, e.g. all those temperature variants. Maybe adding parameters for the requested aggregation or height. If there is a system behind the construction of the parameter names, maybe we could use string template literals to construct them automatically. I am no expert in meteorology, as a hobbyist I am probably fine with a subset of those metrics. We could have standard parameters and advanced parameters, to give beginners an easy start. |
Sure, adding more variables in future package versions is an option. The issue is that many variables like This is also the reason why the FlatBuffers response encodes attributes for table VariableWithValues {
variable: Variable;
unit: Unit;
value: float; // Only used for current conditions
values: [float]; // Contains a time series of data
values_int64: [int64]; // Only for sunrise/set as a unix timestamp
altitude: int16;
aggregation: Aggregation;
pressure_level: int16;
depth: int16;
depth_to: int16;
ensemble_member: int16;
previous_day: int16;
} A solution could be to change how weather variables are requested. Dummy code: let temperature_2m = OmVariable(variable: .temperature, altitude: 2)
let wind_speed_2m = OmVariable(variable: .wind_speed, altitude: 10)
let temperature_2m_max = OmVariable(variable: .temperature, altitude: 2, aggregation: .max)
let temperature_2m_min = OmVariable(variable: .temperature, altitude: 2, aggregation: .min)
// Translates `OmVariable` to a string `temperature_2m` (inefficient!)
let params = Params(
latitude: 12,
longitude: 34,
hourly: [temperature_2m, wind_speed_2m],
daily: [temperature_2m_max, temperature_2m_min]
)
let results = api.fetch(url, params)
// `.get()` matches all attributes like variable, altitude, aggregation, pressure_levels and returns a potential match
let temperature_2m_data = results.hourly.get(temperature_2m)
let wind_speed_2m_data = results.hourly.get(wind_speed_2m)
let temperature_2m_max_data = results.daily.get(temperature_2m_max)
let temperature_2m_min_data = results.daily.get(temperature_2m_min)
// Note: the Ensemble API returns multiple temperature_2m results with different `ensemble_member` attributes from 0...51.
let temperature_2m_ensembles= results.hourly.getMultiple(temperature_2m)
/// Note: Technically different weather domains could be encoded as well, but requires larger changes to the API backend
let temperature_2m_gfs013 = OmVariable(variable: .temperature, altitude: 2, domain: .gfs013)
let temperature_2m_hrrr = OmVariable(variable: .temperature, altitude: 2, domain: .hrrr) This could be more scalable. For this implementation a different API endpoint would be useful to accept the attribute-based variable definition directly instead of string parsing or encode the request using FlatBuffers. The benefit is, that all enumerations from the FlatBuffers definitions can be used directly. The drawback is, that it requires more logic for each supported programming language. If I only consider the struct MyWeatherData {
@Variable(variable: .temperature, altitude: 2)
let temperature2m: [Float]
@Variable(variable: .precipitation)
let preciptation: [Float]
}
let params = Params(
latitude: 12,
longitude: 34,
hourlyClass: MyWeatherData.self
)
let results = api.fetch(url, params)
print(results.hourly.temperature2m) I am not sure which is the right path to take forward. The functionality and variety of data of the API changed quite a lot over the last years. Keeping everything consistent is quite a hard job. |
I understand what you mean with variable count explosion. Just a few thoughts:
I am quite happy with my approach of offering only a solution for a simple forecast use case. Thanks to the naming conventions I parsed the variable names from the website and excluded variables that I don't understand (collapsed parameters, especially temperature per pressure level ;-) |
Feature Request
This package does not really utilize TypeScript as the title of the package promises.
The parameters of the request are of type
any
, so the IDE can't help with autocompletion andthe compiler can't check if the parameters are valid. Also the response of the client seems to
be just the unprocessed flatbuffers object from
@openmeteo/sdk
without any processing.This has a few problems:
In the examples this leads to lots of
!
non-null assertions sprinkled over the code, which istypically an indicator for sub optimal typing.
I propose a solution, that abstracts away the flatbuffers objects and instead returns something that is
typed as a subset from the JSON response of the API. Converting is done internally, only "normal" JavaScript
types like
number
andstring
are used and you don't need to manage indices yourself:I have created a PoC package at @atombrenner/openmeteo so you can play around with my idea a bit.
The text was updated successfully, but these errors were encountered: