Skip to content

Generate definition pvs for stat vars - #602

Merged
n-h-diaz merged 15 commits into
datacommonsorg:masterfrom
n-h-diaz:def
Jul 2, 2026
Merged

Generate definition pvs for stat vars#602
n-h-diaz merged 15 commits into
datacommonsorg:masterfrom
n-h-diaz:def

Conversation

@n-h-diaz

@n-h-diaz n-h-diaz commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Definitions are a serialization of the important pvs for a stat var

These were previously generated in prophet and are required for NL search. Currently backfilled in spanner, but should be added to ingestion

@codacy-production

codacy-production Bot commented Jun 30, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 16 complexity

Metric Results
Complexity 16

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces automatic definition generation for StatisticalVariable nodes in GraphTransformer.java by extracting relevant properties (such as measurement denominator, qualifier, stat type, measured property, population type, and constraints) and formatting them into a comma-separated string. It also adds a helper utility getFirstPropertyValue in McfUtil.java and updates corresponding unit tests. The review feedback suggests several improvements: utilizing existing vocabulary constants instead of hardcoded string literals, ensuring the newly generated definition property is excluded from being classified as a constraint property, and broadening the parameter type in McfUtil.getFirstPropertyValue to PropertyValuesOrBuilder for better flexibility.

Comment thread pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphTransformer.java Outdated
Comment thread pipeline/util/src/main/java/org/datacommons/ingestion/util/GraphTransformer.java Outdated
Comment thread util/src/main/java/org/datacommons/util/McfUtil.java Outdated
@n-h-diaz

n-h-diaz commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces automatic generation of definitions for Statistical Variables within the MCF mutation process. It updates McfMutator to construct a definition string based on properties like measurement denominator, qualifier, stat type, measured property, population type, and constraints, and adds corresponding test coverage. Additionally, several new vocabulary constants are registered. Feedback suggests refactoring generateSVDefinition in McfUtil to accept PropertyValuesOrBuilder for better flexibility and to sort constraint keys internally to guarantee deterministic output regardless of the map implementation passed by the caller.

Comment thread util/src/main/java/org/datacommons/util/McfUtil.java
@n-h-diaz
n-h-diaz merged commit 7a50766 into datacommonsorg:master Jul 2, 2026
11 checks passed
@n-h-diaz
n-h-diaz deleted the def branch July 2, 2026 01:03
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.

2 participants