Repository navigation
fix: keep missing text values None under pandas 3 - #1149
Conversation
A person field that is empty for only some of the sampled people is None in the derived fields and in the output. pandas 3 holds a missing string as NaN, which is truthy and not valid JSON: an en_SG person without a middle name made the email address fail with a TypeError, and en_US bachelors_field came back as NaN. Fixes #1146 Signed-off-by: Yev Meyer <ymeyer@nvidia.com>
 [Medium impact] The change reaches several data-generation and processing paths. Restore the conversion of missing numeric person fields before merging. Findings
Fix with agent prompt### Issue 1
packages/data-designer-engine/src/data_designer/engine/sampling_gen/people_gen.py:149
`PeopleGenFromDataset.generate` now leaves missing numeric fields as NaN. The removed loop converted missing values in every column, but `dataframe_to_records` only converts text/object columns. If a managed dataset has a partly empty numeric field such as `monthly_income_eur`, sampled people receive NaN instead of `None`. Saving that person column with `storage_encoding='json'` then fails because JSON storage rejects non-finite numbers.
Keep the all-column conversion for person records while leaving the shared helper's numeric behavior unchanged.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThe PR adds
|
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
I found no Critical or Warning issues.
I reviewed both changed files and traced the managed dataset reader, record conversion, derived fields, and sampler output. The normalization runs before derived-field generation, preserves populated values, and retains the existing omission of entirely null columns. The regression test exercises the real Parquet/DuckDB path and checks both missing and populated values alongside adult email generation.
Validation was source-only. I didn't execute tests or independently verify the reported pandas 2/3 results.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
A row built from a DataFrame for generation holds None for a missing value in a text or object column, as it does under pandas 2. pandas 3 holds that value as NaN (pd.NA for the string dtypes), which Jinja's `is none` test rejects: an expression or prompt that checked a missing seed value took the wrong branch, and a custom column saw NaN. dataframe_to_records does the conversion for seed and batch results, the row buffer, expression columns, the schema transform processor and the person sampler, which now uses it in place of its own loop. Numeric, datetime and nested values are unchanged. Fixes #1148 Signed-off-by: Yev Meyer <ymeyer@nvidia.com>
3mei
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return [ | ||
| generate_and_insert_derived_fields(p) | ||
| for p in self._generate_from_dataset(n, **kwargs).to_dict(orient="records") | ||
| for p in dataframe_to_records(self._generate_from_dataset(n, **kwargs)) |
There was a problem hiding this comment.
PeopleGenFromDataset.generate now leaves missing numeric fields as NaN. The removed loop converted missing values in every column, but dataframe_to_records only converts text/object columns. If a managed dataset has a partly empty numeric field such as monthly_income_eur, sampled people receive NaN instead of None. Saving that person column with storage_encoding='json' then fails because JSON storage rejects non-finite numbers.
Keep the all-column conversion for person records while leaving the shared helper's numeric behavior unchanged.
Knowledge Base Used: Sampling and synthetic generation
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/data-designer-engine/src/data_designer/engine/sampling_gen/people_gen.py
Line: 149
Comment:
**Missing numeric fields stay NaN**
`PeopleGenFromDataset.generate` now leaves missing numeric fields as NaN. The removed loop converted missing values in every column, but `dataframe_to_records` only converts text/object columns. If a managed dataset has a partly empty numeric field such as `monthly_income_eur`, sampled people receive NaN instead of `None`. Saving that person column with `storage_encoding='json'` then fails because JSON storage rejects non-finite numbers.
Keep the all-column conversion for person records while leaving the shared helper's numeric behavior unchanged.
**Knowledge Base Used:** [Sampling and synthetic generation](https://app.greptile.com/-/switch-tenant?tenant=nvidia-public-github&next=%2Fcustom-context%2Fknowledge-base%2Fnvidia-nemo%2Fdatadesigner%2F-%2Fdocs%2Fsampling-generation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
Fern preview: https://nvidia-preview-pr-1149.docs.buildwithfern.com/nemo/datadesigner
|
📋 Summary
Person sampling from a managed dataset gives
Nonefor a field that is empty for only some of the sampled people. Under pandas 3 these values came through as NaN, so en_SG sampling failed whenever a batch mixed people with and without a middle name, and NaN reached the output.🔗 Related Issue
Fixes #1146
🔄 Changes
PeopleGenFromDataset._generate_from_datasetturns the missing values of each partly empty column intoNonebefore any field is derived. The email address then treats a missing middle name as absent, and the output holds no NaN.test_managed_sampling_gives_none_for_a_value_only_some_people_havesamples a four-person en_SG dataset in whichmiddle_nameandindustryare empty for some people. It reads the dataset throughLocalPersonReaderand DuckDB, as a run does.🧪 Testing
make testpassesmake test(pandas 3.0.5, the locked version):make check-all:Without the change, the new test fails the way the issue describes:
With pandas 2.3.3,
test_people_gen.pyalso passes (20 passed).✅ Checklist
triaged