fix codelist operations type inference for empty dataframes#1709
Conversation
RamilCDISC
left a comment
There was a problem hiding this comment.
Would it be better to add a unit or regression test for this case?
Yes, I added unit tests that fail on main but pass on this branch |
| self.params.ct_package_type, unique_ct_versions | ||
| ) | ||
| ct_df = self.evaluation_dataset.__class__.from_dict(ct_data) | ||
| ct_df = ct_df.astype( |
There was a problem hiding this comment.
The PR only converts the ct_df to string but evaluation dataset is still not made safe for merge. if it has empty columns the merge can fail. I have noticed that in the test the evaluation dataset was made safe manually by converting it to string. That let me to think that we need to make the evaluation dataset safe here too to merge without errors.
There was a problem hiding this comment.
i believe that the fix for the types allowed us to have empty evaluation datasets as well. I've removed the conversions from the tests.
| ct_df = self.evaluation_dataset.__class__.from_dict(ct_data) | ||
| ct_df = ct_df.astype( | ||
| { | ||
| "version": str, |
There was a problem hiding this comment.
using str to covnert to string here can convert missing values like nan to "nan" which could create issue in downstream flow. One approach can be to convert like:
`{
"version": "string",
"codelist_code": "string"
}
`
| ct_df = self.evaluation_dataset.__class__.from_dict(ct_data) | ||
| ct_df = ct_df.astype( | ||
| { | ||
| "version": str, |
…s-in-usdm-rule-execution
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR fixes the bug for empty dataframe in codelist operations. The PR was validated by:
- Reviewing the PR for any unwanted code or comments.
- Reviewing the PR in accordance with the AC.
- Reviewing the updated tests for logic and coverage.
- Reviewing the code for maintaining quality and downstream safe checks.
- Running validation using positive dataset in dev editor.
- Running validation using negative dataset in dev editor.
- Running validation using edge case of empty dataframe.
- Ensuring all unit and integration testing pass.
* Define dependencies in pyproject.toml Moves dependency constraints to pyproject.toml. Makes requirements.txt a lockfile. * Support click 8.3.0 Fixes an incompatibility caused by click 8.3.0, which passes the default value as-is. * Support pyreadstat 1.2.9 Fixes an incompatibility caused by pyreadstat 1.2.9, which changed original_variable_type from 'NULL' to None * Support jsonpath-ng 1.8.0 Works around an behavior change in jsonpath-ng 1.8.0 where Child.str gets wrapped in parenthesis. * Suport dask 2024.8.1 Fixes tokenization errors when using dask 2024.8.1+. Starting with this version, dask enforces that tokens remain stable across pickle round-trips (dask/dask#11320). Capturing self in a lambda fails this check because instance objects can have non-deterministic pickle representations. Since calculate_variable_value_length is already a static method, replacing self with the class name is enough to remove the capture. * Support dask 2024.12.1 Fixes an import error caused by dask 2024.12.1, which removed the legacy dask.dataframe.dd submodule (dask/dask#11604). Changes the import to `import dask.dataframe as dd`, consistent with every other file in the codebase. * Support dask 2025.4.0 Dask 2025.4.0 optimizes multiple DataFrames together, which exposes division mismatches when assigning a pandas Series to a dask DataFrame column. The old reset_index/set_index workaround no longer avoids this. Replacing it with compute-assign-rewrap via dd.from_pandas, which builds a clean expression graph. This is safe because __getitem__ already computes the DataFrame to produce the Series being assigned. * Support pandas 2.2.0 Fixes a unit test to support pandas 2.2.0+. The pandas release fixes a sorting bug with pandas-dev/pandas#54611. This commit changes the expected results accordingly. Also fixes a merge type mismatch introduced by upstream #1709: the codelist metadata side was cast to StringDtype but the evaluation dataset side was not. With pandas 2.2.0, empty columns infer as float64, and merging float64 with string is rejected. Casting both sides to string before the merge resolves this. --------- Co-authored-by: Samuel Johnson <96841389+SFJohnson24@users.noreply.github.com>
* Define dependencies in pyproject.toml Moves dependency constraints to pyproject.toml. Makes requirements.txt a lockfile. * Support click 8.3.0 Fixes an incompatibility caused by click 8.3.0, which passes the default value as-is. * Support pyreadstat 1.2.9 Fixes an incompatibility caused by pyreadstat 1.2.9, which changed original_variable_type from 'NULL' to None * Support jsonpath-ng 1.8.0 Works around an behavior change in jsonpath-ng 1.8.0 where Child.str gets wrapped in parenthesis. * Suport dask 2024.8.1 Fixes tokenization errors when using dask 2024.8.1+. Starting with this version, dask enforces that tokens remain stable across pickle round-trips (dask/dask#11320). Capturing self in a lambda fails this check because instance objects can have non-deterministic pickle representations. Since calculate_variable_value_length is already a static method, replacing self with the class name is enough to remove the capture. * Support dask 2024.12.1 Fixes an import error caused by dask 2024.12.1, which removed the legacy dask.dataframe.dd submodule (dask/dask#11604). Changes the import to `import dask.dataframe as dd`, consistent with every other file in the codebase. * Support dask 2025.4.0 Dask 2025.4.0 optimizes multiple DataFrames together, which exposes division mismatches when assigning a pandas Series to a dask DataFrame column. The old reset_index/set_index workaround no longer avoids this. Replacing it with compute-assign-rewrap via dd.from_pandas, which builds a clean expression graph. This is safe because __getitem__ already computes the DataFrame to produce the Series being assigned. * Support pandas 2.2.0 Fixes a unit test to support pandas 2.2.0+. The pandas release fixes a sorting bug with pandas-dev/pandas#54611. This commit changes the expected results accordingly. Also fixes a merge type mismatch introduced by upstream #1709: the codelist metadata side was cast to StringDtype but the evaluation dataset side was not. With pandas 2.2.0, empty columns infer as float64, and merging float64 with string is rejected. Casting both sides to string before the merge resolves this. * testing * restore * parentheses strip * stringg to str * black formatting for 26.5.1 * pylock * sync changes, and remove requirements * system flag * no build * pyproject.toml, removes requirements * dep ranges * dask, path rework * full path * str * jsonschema deps and tester * path * path * type coercion fix builder * to dict refactor * dask test * line length * azure-functions as optional-dependency --------- Co-authored-by: Filipp Shpomer <filipps@verily.com> Co-authored-by: Gerry Campion <gcampion@cdisc.org>
Fixes the issue for CORE-000857 with #1651. I cannot reproduce the issue for CORE-000873, CORE-000874 without the proper data.
Previously, we would get an Execution Error because the operations input dataframe is empty (after match datasets) and the column datatypes cannot be inferred.
Broken.xlsx
Now we get a skip.
Fixed.xlsx
Ran with following data and launch config:
pilot_LZZT_narrative_2026MAR10.json
{ "name": "USDM Rule", "type": "debugpy", "request": "launch", "program": "${workspaceFolder}/core.py", "console": "integratedTerminal", "args": [ "validate", "-s", "usdm", "-v", "4-0", "-dp", "pilot_LZZT_narrative_2026MAR10.json", "-r", "CORE-000857", "-of", "XLSX", "-of", "json", "-l", "debug" ] }