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

Reading invalid formatted plifile should raise error instead of warning #320

Closed
veenstrajelmer opened this issue Aug 19, 2022 · 4 comments · Fixed by #457
Closed

Reading invalid formatted plifile should raise error instead of warning #320

veenstrajelmer opened this issue Aug 19, 2022 · 4 comments · Fixed by #457
Assignees
Labels
type: enhancement Improvements to existing functionality
Milestone

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Aug 19, 2022

Describe the bug
Reading invalid formatted plifile should raise error instead of warning.

To Reproduce

polyfile_object = read_polyfile(file_pli,has_z_values=False)

Expected behavior
Case with comments added (not supported by hydrolib, but no issue for dflowfm kernel). I would say a # is necessary but also that is not supported.

DCSM-FM_OB_all_20181108
    5  2
-9.250000000000000E+000  4.300000000000000E+001 DCSM-FM_OB_all_20181108_0001
-9.500000000000000E+000  4.300000000000000E+001 DCSM-FM_OB_all_20181108_0002
-9.750000000000000E+000  4.300000000000000E+001 DCSM-FM_OB_all_20181108_0003
-1.000000000000000E+001  4.300000000000000E+001 DCSM-FM_OB_all_20181108_0004
-1.025000000000000E+001  4.300000000000000E+001 DCSM-FM_OB_all_20181108_0005

UserWarning: Expected a valid next point at line 2.
should raise exception of invalid formatted plifile

Case with invalid nrows in plifile

DCSM-FM_OB_all_20181108
    206    2
-9.250000000000000E+000  4.300000000000000E+001 
-9.500000000000000E+000  4.300000000000000E+001 
-9.750000000000000E+000  4.300000000000000E+001 
-1.000000000000000E+001  4.300000000000000E+001 
-1.025000000000000E+001  4.300000000000000E+001 

UserWarning: EoF encountered before the block is finished.
should raise exception of invalid formatted plifile

Case with valid formatted pli file

DCSM-FM_OB_all_20181108
    5   2
-9.250000000000000E+000  4.300000000000000E+001 
-9.500000000000000E+000  4.300000000000000E+001 
-9.750000000000000E+000  4.300000000000000E+001 
-1.000000000000000E+001  4.300000000000000E+001 
-1.025000000000000E+001  4.300000000000000E+001 

UserWarning: White space at the start of the line is ignored.
this is not to relevant to warn about, it is about the white space before '5'. Disable warning?
for the last thing, I created a separate issue: #370

Version info (please complete the following information):

  • OS: Windows, Anaconda, Spyder
  • Version: 0.3.0
@arthurvd arthurvd added this to To do in HYDROLIB-core via automation Sep 20, 2022
@arthurvd arthurvd added this to the Release 0.4 milestone Sep 20, 2022
@veenstrajelmer
Copy link
Collaborator Author

veenstrajelmer commented Oct 3, 2022

Would be nice if the last point can be resolved before 0.4, maybe as part of #34 ? Otherwise, I made a PR here: #359

@arthurvd arthurvd modified the milestones: Release 0.4, Release 0.5 Jan 23, 2023
@arthurvd arthurvd added the type: enhancement Improvements to existing functionality label Mar 7, 2023
@MRVermeulenDeltares MRVermeulenDeltares moved this from To do to In progress in HYDROLIB-core Mar 8, 2023
@MRVermeulenDeltares
Copy link
Contributor

Case with valid formatted pli file & UserWarning: White space at the start of the line is ignored, already resolved in #370

@MRVermeulenDeltares
Copy link
Contributor

MRVermeulenDeltares commented Mar 8, 2023

Discussed with Jelmer: In all cases in which the file is invalid, an exception error should be raised, instead of a warning

MRVermeulenDeltares added a commit that referenced this issue Mar 10, 2023
@MRVermeulenDeltares MRVermeulenDeltares moved this from In progress to Ready to review in HYDROLIB-core Mar 10, 2023
@tim-vd-aardweg tim-vd-aardweg moved this from Ready to review to Review in progress in HYDROLIB-core Mar 13, 2023
@tim-vd-aardweg tim-vd-aardweg moved this from Review in progress to Reviewer follow up in HYDROLIB-core Mar 13, 2023
@MRVermeulenDeltares MRVermeulenDeltares moved this from Reviewer follow up to In progress in HYDROLIB-core Mar 13, 2023
@MRVermeulenDeltares MRVermeulenDeltares moved this from In progress to Ready to review in HYDROLIB-core Mar 13, 2023
@tim-vd-aardweg tim-vd-aardweg moved this from Ready to review to Review in progress in HYDROLIB-core Mar 13, 2023
@tim-vd-aardweg tim-vd-aardweg self-assigned this Mar 13, 2023
@tim-vd-aardweg tim-vd-aardweg moved this from Review in progress to Test in HYDROLIB-core Mar 13, 2023
@arthurvd arthurvd moved this from Test to Done in HYDROLIB-core Mar 13, 2023
@priscavdsluis priscavdsluis moved this from Done to Test in HYDROLIB-core Mar 14, 2023
HYDROLIB-core automation moved this from Test to Done Mar 15, 2023
@arthurvd
Copy link
Member

arthurvd commented Mar 15, 2023

For completeness: @veenstrajelmer and @MRVermeulenDeltares : this morning we were discussing that for now we will NOT support reading of support point labels at the end of a data line (but don't raise an error if they are present). Coincidentally, @xldeltares has reported that she is hampered by that, see new issue #467. I will continue discussion over there and keep this issue closed (because this issue here was only about changing warning into error, regardless of what we do and do not support in pli files)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvements to existing functionality
Projects
HYDROLIB-core
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants