Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional location-related fields should not be written when empty #104

Closed
arthurvd opened this issue Oct 22, 2021 · 3 comments · Fixed by #336
Closed

Optional location-related fields should not be written when empty #104

arthurvd opened this issue Oct 22, 2021 · 3 comments · Fixed by #336
Assignees
Labels
type: bug Something isn't working
Milestone

Comments

@arthurvd
Copy link
Member

arthurvd commented Oct 22, 2021

In the .ext file, locations for Lateral and Boundary objects can be specified in multiple ways, so all these fields are optional.
When an MDU+ext file is saved to disk, also all optional fields are written, which is not an error, but it is unnecessarily confusing, nonetheless.
Example .ext input:

[Lateral]
	id = lat_871
	name = lat_871
	type = discharge
	locationType = 1d
	branchId = riv_RS1_909
	chainage = 99.678
	discharge = boundaries.bc

[Boundary]
	quantity = waterlevelbnd
	nodeId = 141133.805000_395446.430000
	forcingFile = boundaries.bc

Currently saved output:

[Lateral]
id             = lat_871
name           = lat_871
locationType   = 1d
nodeId         =  # <<< May be omitted
branchId       = riv_RS1_909
chainage       = 99.678
numCoordinates =  # <<< May be omitted
xCoordinates   =  # <<< May be omitted
yCoordinates   =  # <<< May be omitted
discharge      = boundaries.bc
type           = discharge

[Boundary]
quantity     = waterlevelbnd
nodeid       = 141133.805000_395446.430000
locationfile =  # <<< May be omitted
forcingfile  = d:\dam_ar\dflowfm_models\HYDROLIB\MGB_FMonly\fm\saved\boundaries.bc
bndwidth1d   =  # <<< May be omitted
bndbldepth   =  # <<< May be omitted

Note that the same issue arises for Structures (locations) and DamBreaks (waterlevel locations)

@arthurvd arthurvd added the type: enhancement Improvements to existing functionality label Oct 22, 2021
@arthurvd arthurvd added this to To do in HYDROLIB-core via automation Oct 22, 2021
@arthurvd arthurvd moved this from To do to In progress in HYDROLIB-core Jul 19, 2022
@arthurvd arthurvd self-assigned this Jul 19, 2022
@priscavdsluis priscavdsluis added this to the Release 0.4 milestone Aug 1, 2022
@priscavdsluis
Copy link
Contributor

@rhutten, do you know whether it is "safe" to skip empty properties for all files?
The MDU is the only INI file I can think of that is OK to write empty properties.
Please verify for which files this change can or cannot be implemented.

@priscavdsluis
Copy link
Contributor

@arthurvd, I will change it to a bug, because we should not be serializing models to file that will raise error when deserialized again. Hope you agree with this :)

@priscavdsluis priscavdsluis added type: bug Something isn't working and removed type: enhancement Improvements to existing functionality labels Sep 6, 2022
priscavdsluis added a commit that referenced this issue Sep 8, 2022
…r mdu and to True for the rest of the files.
@priscavdsluis priscavdsluis moved this from In progress to Ready to review in HYDROLIB-core Sep 8, 2022
@priscavdsluis
Copy link
Contributor

@arthurvd and @rhutten, I have made the assumption that empty properties should be written for the MDU file and they should be skipped for all other files. Please someone verify this.
Putting this issue for review.

priscavdsluis added a commit that referenced this issue Sep 8, 2022
@arthurvd arthurvd moved this from Ready to review to Review in progress in HYDROLIB-core Oct 5, 2022
@arthurvd arthurvd moved this from Review in progress to Reviewer follow up in HYDROLIB-core Oct 5, 2022
@priscavdsluis priscavdsluis moved this from Reviewer follow up to In progress in HYDROLIB-core Oct 10, 2022
HYDROLIB-core automation moved this from In progress to Done Oct 13, 2022
@priscavdsluis priscavdsluis modified the milestones: Release 0.4, 0.3.1 Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
HYDROLIB-core
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants