unskipped setup_gauges and fixed issue with csv file reading

[Tjalling-dejong]

Issue addressed

Fixes Deltares/hydromt#1243

Part of process of unblocking #347

Explanation

data_catalog.get_geodataframe() was missing a driver argument when using a gauges table file (.csv, .xlsx, etc.). By default get_geodataframe uses the pyogrio driver to read the data while it should be a geodataframw_table driver.

I also updated the update_model_gauges.ipynb notebook to v1.

Checklist

• Updated tests or added new tests
• Branch is up to date with main
• Tests & pre-commit hooks pass
• Updated documentation if needed

Additional Notes (optional)

Add any additional notes or information that may be helpful.

[hboisgon]

Nice! Thanks for digging up @Tjalling-dejong

I'm not sure I'm happy about the solution though... I feel like this is not something straighforward for the users to pass the driver argument. Also the syntax is not so fun and I'm pretty sure not well documented...
Anyhow I now realised we really changed something in v1. In v10, we had both a default driver and then default drivers linked to specific file extensions. See for example:

https://github.com/Deltares/hydromt/blob/e5c93aed382065c6db60dd0d0f6d100eaa0c46bd/hydromt/data_adapter/geodataframe.py#L41-L48

But this is true for all data source types (also rasterdataset etc). So I for example guess that if I pass only a link to a netcdf file, I'll have to specify manually the driver like what is done here. Personnally I would be in favor to update v1 to do what it used to: try to infer driver from file extension (with good debug message that driver not found, assuming ...). What do you think?

[Tjalling-dejong]

@hboisgon

At first I did agree with you on the inferring of the driver by hydromt core, but after looking through the core code I would say it is the plugins responsibility to pass on the driver argument. The drivers in hydromt core are set up in such a way that you can add your own custom drivers. Core can then not know which custom driver belongs to which file type. I therefore think that is the plugins responsibility to pass on which driver is to be used.

For this particular case with setup_gauges I can include an optional driver key word argument that if not specified the driver is inferred from the file extension. See latest commit

[hboisgon]

@hboisgon

At first I did agree with you on the inferring of the driver by hydromt core, but after looking through the core code I would say it is the plugins responsibility to pass on the driver argument. The drivers in hydromt core are set up in such a way that you can add your own custom drivers. Core can then not know which custom driver belongs to which file type. I therefore think that is the plugins responsibility to pass on which driver is to be used.

For this particular case with setup_gauges I can include an optional driver key word argument that if not specified the driver is inferred from the file extension. See latest commit

I disagree. This is only default behavior. If custom drivers are needed, I think a data catalog entry will be required and files won't be able to be read from path. If it's for another reason that the default drivers cant be used then the plugins will indeed take care of it. In a way if we do not support this flexibility in the driver I wonder why even bother supporting Path as an option? Then just data_catalog or direct python object. I think this is a shame to loose this for the user but it also does not make sense to only half support it.
Let's take it to the group at the next standup.