Skip to content

fix codelist operations type inference for empty dataframes#1709

Merged
gerrycampion merged 5 commits into
mainfrom
1651-new-version-of-core-engine-causes-errors-in-usdm-rule-execution
Apr 28, 2026
Merged

fix codelist operations type inference for empty dataframes#1709
gerrycampion merged 5 commits into
mainfrom
1651-new-version-of-core-engine-causes-errors-in-usdm-rule-execution

Conversation

@gerrycampion

Copy link
Copy Markdown
Collaborator

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"
  ]
}

@RamilCDISC RamilCDISC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to add a unit or regression test for this case?

@gerrycampion

Copy link
Copy Markdown
Collaborator Author

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(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here too

@RamilCDISC RamilCDISC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR fixes the bug for empty dataframe in codelist operations. The PR was validated by:

  1. Reviewing the PR for any unwanted code or comments.
  2. Reviewing the PR in accordance with the AC.
  3. Reviewing the updated tests for logic and coverage.
  4. Reviewing the code for maintaining quality and downstream safe checks.
  5. Running validation using positive dataset in dev editor.
  6. Running validation using negative dataset in dev editor.
  7. Running validation using edge case of empty dataframe.
  8. Ensuring all unit and integration testing pass.

@gerrycampion
gerrycampion merged commit 4d89e6d into main Apr 28, 2026
13 of 14 checks passed
@gerrycampion
gerrycampion deleted the 1651-new-version-of-core-engine-causes-errors-in-usdm-rule-execution branch April 28, 2026 15:41
SFJohnson24 added a commit that referenced this pull request Jun 5, 2026
* 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>
SFJohnson24 added a commit that referenced this pull request Jun 15, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New version of Core engine causes errors in USDM rule execution

2 participants