search

nens / threedi-schema

The schema of 3Di schematization files
MIT License
0 stars 0 forks source link

Some fields should be boolean instead of integer #130

Open leendertvanwolfswinkel opened 1 month ago

leendertvanwolfswinkel commented 1 month ago

model_settings.friction_averaging model_settings.use_2d_rain physical_settings.use_advection_1d physical_settings.use_advection_2d

margrietpalm commented 5 days ago

@leendertvanwolfswinkel model_settings.friction_averaging is a custom enum type:

friction_averaging = Column(IntegerEnum(constants.OffOrStandard))

Any specific reason to make this a bool? Because this is quite a bit of extra work compared to the other three.

margrietpalm commented 5 days ago

@leendertvanwolfswinkel physical_settings.use_advection_1d has several options: 

use_advection_1d = Column(IntegerEnum(constants.AdvectionTypes1D))

with

class AdvectionTypes1D(Enum):
    OFF = 0
    MOMENTUM_CONSERVATIVE = 1
    ENERGY_CONSERVATIVE = 2
    COMBINED_MOMENTUM_AND_ENERGY_CONSERVATIVE = 3

I do remember changing, this but do not remember exactly why.

leendertvanwolfswinkel commented 4 days ago

OK I may have been a little bit too boolean-happy here...

use_advection_1d indeed has several options, so should not be converted to boolean. It is also somewhat likely that we add more options to use_advection_2d sometime in the future.

However, for model_settings.friction_averaging and model_settings.use_2d_rain I don't think we will add extra options, so imo boolean would be more suitable.