WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@lcoram
Copy link
Collaborator

@lcoram lcoram commented Dec 5, 2025

  • Return 404 when idf data not found (idf_handler), not 500
  • Seems that its impossible to actually put a column that is an array into csv (with the rust csv writer), also nested structs aren't supported: CsvError(Error(Serialize("serializing maps is not supported, if you have a use case, please file an issue at https://github.com/BurntSushi/rust-csv"))). So supporting creating these arrays for duration and frequencies is a bit of a hack, keeping it in the metadata file as a string, then parsing it back out.
    {
    "stationId": 18165,
    "numberOfSeasons": 11,
    "fromTime": "2013-02-23",
    "toTime": "2024-12-31",
    "qualityClass": 3,
    "seedParameter": 1,
    "updatedAt": "2025-01-01",
    "durations": [10, 20, 30, 100, ...],
    "frequencies": [2, 5, 10, 20, 50, ...],
    },
    ...
    instead of just
    {
    "stationId": 18165,
    "numberOfSeasons": 11,
    "fromTime": "2013-02-23",
    "toTime": "2024-12-31",
    "qualityClass": 3,
    "seedParameter": 1,
    "updatedAt": "2025-01-01"
    },

@lcoram lcoram marked this pull request as ready for review December 5, 2025 12:47
@lcoram lcoram force-pushed the fixes_to_reports_egress branch from f82cef6 to a23999c Compare December 5, 2025 13:24
@Lun4m
Copy link
Collaborator

Lun4m commented Dec 8, 2025

Why do we need this? It's not like you can select only specific durations/frequencies?

@jo-asplin-met-no
Copy link
Collaborator

That's exactly what you should be able to do. The set of available durations and frequencues can generally vary between stations, and front-end code can make use of this to construct GUIs.

@jo-asplin-met-no
Copy link
Collaborator

Or maybe I'm wrong ...

@jo-asplin-met-no
Copy link
Collaborator

Maybe the available sets of durations and frequencies are the same for all stations in practice. But even with that assumption, it is a problem that those sets aren't provided by the availability endpoint at all (/reports/idf/station). That's a showstopper, since how can Frost then even tell what values to choose from?

@jo-asplin-met-no
Copy link
Collaborator

Providing those sets per station at least allows us to easily (i.e. automatically!) support varying sets in the future even if they in practice don't vary currently (and thus constitute redundancy in the response). Maybe check with Ketil?

@lcoram
Copy link
Collaborator Author

lcoram commented Dec 8, 2025

https://frost.met.no/api.html#!/frequencies_(DEPRECATED)/getRainfallIDF
https://frost-rc.met.no/docs/apirefadvanced#/idf%2Fstation/idfStationBase
Frost v0 allowed you to do this, and so it was kinda a regression in v1... which I think Jo is trying to fix?

@jo-asplin-met-no
Copy link
Collaborator

jo-asplin-met-no commented Dec 8, 2025

I think specifying a subset of both durations and frequencies for a given station has been a requirement to Frost all along. Note that it is ok if the back-end returns intensity values for all available dur/freq combinations, as Frost is just skipping the ones you don't ask for in the final response. The default is to not filter, i.e. return for all available values.

@Lun4m
Copy link
Collaborator

Lun4m commented Dec 8, 2025

As far as I know it's always the same set and it's only 128 duration/frequency pairs per station, but I guess it could increase in the future? If filtering of duration/frequency is desired anyway, I feel like it should be implemented here as well?

@lcoram lcoram marked this pull request as draft December 10, 2025 12:33
@lcoram
Copy link
Collaborator Author

lcoram commented Dec 10, 2025

Waiting to see if the front end actually needs this.

@lcoram lcoram added the wontfix This will not be worked on label Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants