Fixed published open rules for usdm#1759
Conversation
| var: (int(length) if pd.notna(length) else None) | ||
| var: ( | ||
| int(length) | ||
| if pd.notna(length) and (isinstance(length, int) or length.isdigit()) |
There was a problem hiding this comment.
If a column has a missing/blank value in csv, it can cause pandas to read the whole column as float instead of int. So for row where value is present the check of length.isdigit() will cause error because floats don't have isdigit()
There was a problem hiding this comment.
why would it read the whole column as float?
There was a problem hiding this comment.
Here on pandas documentation it states it: link
There was a problem hiding this comment.
The value in the fixture now is changed to be 'patients' from 'patients.csv'. This line here still replaces the value 'patients.csv' with its upper case version, which will basically update nothing. This defeats the test purpose judging by its name.
SFJohnson24
left a comment
SFJohnson24
left a comment
There was a problem hiding this comment.
See comment -- there is a discrepency in issues produced with standard name when capitalized vs lowercase. When I add a lower() call to the standard, it reverts back to the lowercase 3 issues.
| ) | ||
| df = reader.from_file(full_path) | ||
| # Build a simulated json pointer for the case where we are simulating json data. | ||
| if self.standard == "usdm": |
There was a problem hiding this comment.
I think this and the excel data service using string literals for usdm are a bit fragile-- I ran a validation using USDM in capitals versus usdm in lowercase and got different results for the same data.
There was a problem hiding this comment.
This was actually a bug with the recently-added caching. I've resolved the bug and you should now see consistent results. Let me know if you still have other concerns about the capitalization.
@SFJohnson24 These files aren't accessible. Can you reattach as .txt files? |
SFJohnson24
left a comment
SFJohnson24
left a comment
There was a problem hiding this comment.
This PR correctly resolves the issues in the attached task. It also resolves the differences in report with the same data but different capitalization
3 participants
Fixes the tasks detailed in the attached issue