Checks in readers/writers to enforce correct types for event_name and event_id container and elements
#951
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
September 25, 2024 09:42
The rationale is to factorize the checking of hazard attribute in readers, for possible future checks than event_name == str
`assert_almost_equals` tries to convert to float which failed for some reason. In any case we want the event_names to be completely equal not almost.
|
Note that I still need to check/decide if additional tests that make sure a warning is raised, or writing fails in the corresponding case is required. Any discussion on this will be appreciated :) |
in order to avoid circular imports
|
@spjuhel Any plan with this PR? Is this ready for review? In this case, please assign a reviewer. Thanks! |
|
Just asked @emanuel-schmid to review as it is slightly technical. I did not add any tests, still unsure whether this would be useful... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this PR:
ImpactandHazardto fail ifevent_nameattribute contains elements that are not strings.Impactreaders (from_excelandfrom_csv), check ifevent_nameelements are all string, and warn user and convert to string otherwise, as reading relies on pandas here, checking and casting also use pandas. (note, that changes tofrom_hdf5were done in Explicitly convertevent_nameto strings inImpact.from_hdf5#894)_check_and_cast_attrmethod inHazardIOwith the purpose of checking the type of both containers and elements of hazard attributes read by the different reader, and casting when required (warning the user in this case).from_raster,from_xarray_raster,from_excelandfrom_csvto effectively enforce correct types.The idea behind implementing
_check_and_cast_attris to factorize the checking/casting and allow for possible additional test on the type of container/elements of other attributes in the future.Currently, it only checks that (when read, i.e., before being set as an attribute of the object):
event_nameis a list of strevent_idis a ndarray (not element type check)This should be considered as a "medium term" temporary solution, before handling attributes such as
event_idevent_namein a DataFrame, which would greatly simplify reading, writing, type checking, etc.This PR fixes #910 #950
PR Author Checklist
develop)PR Reviewer Checklist