hdf loading in BaseConfig

Transport-for-the-North / caf.toolkit

Toolkit of transport planning and appraisal functionalities.

https://caftoolkit.readthedocs.io/en/stable/

Other

0 stars 2 forks source link

hdf loading in BaseConfig #71

Open isaac-tfn opened 1 year ago

isaac-tfn commented 1 year ago

Add functionality to BaseConfig to read/write configs to hdf files. This is implemented in caf.core but it probably makes more sense for it to be a method in BaseConfig as it is likely to be used in more than one class inheriting from BaseConfig

wsp-mbuckley commented 1 year ago

Would these mean that toolkit would then need to depend on pytables or is the implementation just converting baseconfig into an easier format to store in the HDF file?

isaac-tfn commented 1 year ago

The yaml read/write is done with h5py but yes that would need to be added as a dependency. If that's not desirable and/or if this seems like a specific case which won't often be used then the functionality can stay in core.

wsp-mbuckley commented 1 year ago

2 options I can think of:

Optional dependency for toolkit so wrap the h5py in a try / except and just raise an error if the BaseConfig.to_hdf method is called
Sub-class BaseConfig in caf.core and add the functionality there, although this would add caf.core as a dependency for anyone else which required the functionality

isaac-tfn commented 1 year ago

I think it's easier to have an optional dependency in toolkit seen as to make it work you'd have to have h5py installed either way, but in the second option it's more complicated and you need caf.core installed as well

wsp-mbuckley commented 1 year ago

I think it's easier to have an optional dependency in toolkit seen as to make it work you'd have to have h5py installed either way, but in the second option it's more complicated and you need caf.core installed as well

Good point, I agree

BenTaylor-TfN commented 1 year ago

Agreed on all points - optional dependency is the way forward.