ioos / erddapy

Python interface for ERDDAP
https://ioos.github.io/erddapy/
BSD 3-Clause "New" or "Revised" License
76 stars 30 forks source link

Automatically call griddap_initialize if a dataset_id exists #353

Open ocefpaf opened 3 weeks ago

ocefpaf commented 3 weeks ago

Right now we need this extra step for griddap but we can skip it run that automatically every time a dataset_id is set.

ocefpaf commented 3 weeks ago

@callumrollo this is something I wanted to do for a long time but only got to it after your tutorial yesterday.

The PR is not ready yet b/c we need to think about the possible corner cases. Also, setting a griddap dataset_id is slow now b/c griddap_initialize is always called. I'm not sure if there is a better option b/c we allow users to re-call it, making checking if those properties are already defined would prevent re-running... Or we can make a breaking change and throw away re-running griddap_initialize and assume that, if self.constraints, self.dim_names, and self.variables, calling griddap_initialize won't change them.

callumrollo commented 3 weeks ago

Nice! I think calling griddap_initialize in the background when a user sets dataset_id when the server was set up with protocol=griddap is a great idea. I don't think the delay is significant enough to be annoying.

I would recommend against throwing away the user available griddap_initialize method though. As a user I sometimes do something like:

e = ERDDAP(y) e.dataset_id = x e.griddap_initialise (no longer need this, nice!) e.variables = a,b,c e.constraints = {} # some breaking changes e.to_xarray() # fails due to the breaking changes I made e.griddap_initialize() # reset the constraints to something that works again

ocefpaf commented 3 weeks ago

I thought about renaming it to griddap_reset or re_initialize but that would be a breaking change. We will keep it and document that one should use it only to "reset the query params."