Flip outlier indicator and use longer value fields in scripts/export.csv
#115
Conversation
| @@ -20,6 +20,9 @@ | |||
| OUTLIER_TYPE_FIELD = "USER27" | |||
| RUN_ID_FIELD = "USER28" | |||
|
|
|||
| IS_OUTLIER_INDICATOR = "N: Non arms-length" | |||
| NON_OUTLIER_INDICATOR = "Y: Is arms-length" | |||
|
|
|||
| OUTLIER_TYPE_CODES = { | |||
| "Not outlier": "0", | |||
There was a problem hiding this comment.
Do we also want to use the long value for the outlier type, in order to make that field read-only? I notice that the iasWorld data model does not have an "analyst determination reason" field in addition to "model determination reason", as "model determination"/"analyst determination" does, so perhaps we want this field to be writeable, but I also wonder if that was an oversight in our iasWorld schema design.
There was a problem hiding this comment.
We do actually want the determination reason to be writable, as it's basically shared between the model / analyst determination fields.
| & (flag_df[OUTLIER_TYPE_FIELD] == OUTLIER_TYPE_CODES["Not outlier"]) | ||
| ].empty, ( | ||
| f"{OUTLIER_TYPE_FIELD} cannot be {OUTLIER_TYPE_CODES['Not outlier']} " | ||
| f"when {IS_OUTLIER_FIELD} is Y" | ||
| f"{OUTLIER_TYPE_FIELD} cannot be '{OUTLIER_TYPE_CODES['Not outlier']}' " |
There was a problem hiding this comment.
I added enclosing quotation marks for a slightly more readable error message.
| @@ -20,6 +20,9 @@ | |||
| OUTLIER_TYPE_FIELD = "USER27" | |||
| RUN_ID_FIELD = "USER28" | |||
|
|
|||
| IS_OUTLIER_INDICATOR = "N: Non arms-length" | |||
| NON_OUTLIER_INDICATOR = "Y: Is arms-length" | |||
|
|
|||
| OUTLIER_TYPE_CODES = { | |||
| "Not outlier": "0", | |||
There was a problem hiding this comment.
We do actually want the determination reason to be writable, as it's basically shared between the model / analyst determination fields.
| IS_OUTLIER_INDICATOR = "N: Non arms-length" | ||
| NON_OUTLIER_INDICATOR = "Y: Is arms-length" | ||
|
|
There was a problem hiding this comment.
I feel like this has the potential to become incredibly confusing. Let's all align on the correct indicator meaning/direction this afternoon and keep this PR open until then.
|
This PR has been open for a long time and we're planning to do a major refactor to this data model soon anyway, so I'm going to close it for now. |
jeancochrane
deleted the
jeancochrane/make-model-reason-readonly-in-export-script
branch
This PR fixes two problems with
scripts/export.csvintroduced in #114 that we noticed after a test upload:Ycorresponded to outliers andNcorresponded to non-outliers, which is the opposite of the export specModel Determinationfield (YorN) resulted in the field being writeable after exportTesting
Test instructions are the same as #114:
You may have to run
aws-mfaif you haven't already today.