-
Notifications
You must be signed in to change notification settings - Fork 1
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
Portfolio validation fails if limit and deductible type columns are missing #57
Comments
@carlfischerjba - these are conditionally required fields, so the do need to be there is the parent field is present. |
@benhayes21 what's the parent field in this case? But that doesn't explain why we need to supply an empty column. If a value is required, it would make sense to require a column with values but an empty column does nothing to provide those values. If the software is able to use a default value when the column is empty, it should be able to use the same default value when the column is missing. |
Hi @carlfischerjba its more of a sibling than a parent. LocDed{} and LocDedType{} fields are mutually required, and its the same across all hierarchal levels. Its important to include the type fields as a common error is to forget about it and not identify % terms as a type 1 or 2. Fractional numbers get applied as monetary fields, and then the numbers are incorrect and nobody realises. I do see your point though. I don't know what default value would be best to avoid the above situation. Any thoughts? |
If there is no sensible default then shouldn't an empty column be rejected in the same way as a missing column? We're not providing any extra information when adding the extra empty column. Seems like it's an issue with how the validation handles missing values and missing columns rather than a problem with the schema itself (which I'm not well placed to judge). |
The validation just uses the schema definitions though. Ideally the validation will be dumb and just accept what's in the schema - we don't really want it to make its own decisions outside of the schema. For A third option might be to implement some more complex default logic - e.g. default to 0 if (ded > 1) or 1 if (0 < ded <=1) But whatever we do, I'm keen on the schema being the thing that drives the rules and the validation tool applies the schema. |
There are a couple of different things going on.
There's no need to change the schema, just how the schema is interpreted and enforced by the software. |
Issue Description
The limit and deductible type fields have a default value of
0
(treat as an absolute amount) and are therefore not mandatory. However, portfolio validation fails if columns such asLocDedType3Contents
orLocLimitType1Building
are missing. By adding in empty columns, validation passes. Empty column or absent column should be treated the same.Steps to Reproduce (Bugs only)
ods_tools check --location exposure_loc_latlon_TCs.csv
(attached).Expected behaviour: no errors and the default value from the spec is used.
(The error about missing
LocPeril
is a separate issue. It would be convenient if the default was to apply to all perils, or possibly use the value ofLocPerilsCovered
but I admit I'm unclear about the purpose of this field.)Version / Environment information
ods_tools 3.1.0
Example data / logs
exposure_loc_latlon_TCs.csv
The text was updated successfully, but these errors were encountered: