Open efvik opened 1 month ago
Personally I like the simplicity of the TimeSeries class. It is one point, one continuous time range. I agree with a lot of your feature request, but instead of having everything into the constructor, it would in my opinion be beneficial to add several methods. See ongoing PR https://github.com/MET-OM/metocean-api/blob/55c4f48db5788321e5f37084b1a5524556aa37fe/metocean_api/ts/ts_mod.py where I suggest adding a download method. It is just as easy to add a get_urls and or an initialise method that prefetches and does all the setup neded before doing the actual downloads. Doing a "lot of expencive stuff" in the constructor (such as going online) is probably not a good idea because it makes the class less versatile.
I think just accessing a file without downloading anything is very inexpensive, less than a second. And that it would make sense to check that the files exist and the server is available. But I agree that separating the process into disctinct methods is a good idea. A separate "verification" method could be added, to be run manually by users if they want feedback on the data they have selected before downloading. It would be optional and not interfere with the rest of the system. This is a low priority feature though, and I guess it doesn't make sense to add while the project structure is changing.
Initializing the TimeSeries module currently just sets the variables. I think it would be better to restructure and perform most of the preparations at initialization, rather than in import_data. For example:
This way we can also print (optional) feedback before downloading is initialized, for example:
This would make it easier to use the api for someone unfamiliar with a product, and make it possible to debug or adjust the request before starting a download.