search

ec-jrc / pyPoseidon

Framework for Hydrodynamic simulations
https://pyposeidon.readthedocs.io/
European Union Public License 1.2
20 stars 7 forks source link

schism: `to_force()`: Handle `rpath` that doesn't end in a slash #147

Closed pmav99 closed 1 year ago

pmav99 commented 1 year ago

We switch to using os.path.join() instead of using plain string concatenation. The idea here is to try to deal with missing trailing slashes (/) in path definitions.

The way this is implemented, there is a small change in behavior. More specifically filename is always os.path.joined with the "sflux" directory. This can be seen in tests/test_meteo_slice.py where all.nc had to be changed to sflux/all.nc. This may or may not be desirable.

brey commented 1 year ago

In cast:336, 364 maybe we can simplify as we do in cast:423?

pmav99 commented 1 year ago

Sure. BTW, Is there some reason we split files and station_files? Couldn't we merge the lists?

brey commented 1 year ago

station_files are optional (they might not exist) while files are mandatory (should exist)

pmav99 commented 1 year ago

According to https://github.com/pmav99/pyPoseidon/blob/35dcd64c4c920f607db4d60797d0d2dc027944af/pyposeidon/utils/cast.py#L338 we treat files as non-mandatory files, too, i.e. they only get copied if they exist. If they are missing we are moving on to the next file. If they are mandatory, shouldn't we raise an exception instead?

brey commented 1 year ago

Well, 2 of them are required but they are also empty and the other one is a bash construct. I would say we could merge them in one list. The sym_files are the core model and for that we have no exception. However, there are also optional files there, depending on the setup. I think it would become too complicated. Let's keep it functional for our setup for the time being.