Skip to content

Modified manifest json and added goldens - #2179

Open
Krishnam24maheshwari wants to merge 3 commits into
datacommonsorg:masterfrom
Krishnam24maheshwari:india_rbi
Open

Modified manifest json and added goldens#2179
Krishnam24maheshwari wants to merge 3 commits into
datacommonsorg:masterfrom
Krishnam24maheshwari:india_rbi

Conversation

@Krishnam24maheshwari

Copy link
Copy Markdown
Contributor

No description provided.

@google-cla

google-cla Bot commented Aug 24, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@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 adds golden data files, updates the import manifest, and introduces a validation configuration for the India RBI State Domestic Product import. The reviewer feedback highlights incorrect Wikidata ID mappings for Chandigarh and Delhi in the golden observations, which should be updated to match the repository's rules for Indian administrative data. Additionally, it is recommended to adjust the DELETED_RECORDS_PERCENT threshold in the validation configuration from 0.1 to 10 if a 10% deletion limit was intended.

"wikidataId/Q1162"
"wikidataId/Q1164"
"wikidataId/Q1165"
"wikidataId/Q43433"

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.

high

According to the repository's general rules for mapping Indian administrative data at the AA1 level, the correct Wikidata ID for Chandigarh should be Q120971341 (the constituency/government entity) rather than the primary geographical place Q43433. Please update the mapping configuration (e.g., pvmap.csv) to resolve Chandigarh to Q120971341 and regenerate the golden observations.

"wikidataId/Q120971341"
References
  1. When mapping Indian administrative data at the AA1 level, use Q120971341 for Chandigarh, Q9357528 for Delhi, and Q200667 for Ladakh.

"wikidataId/Q1165"
"wikidataId/Q43433"
"wikidataId/Q1168"
"wikidataId/Q1353"

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.

high

According to the repository's general rules for mapping Indian administrative data at the AA1 level, the correct Wikidata ID for Delhi should be Q9357528 rather than the primary geographical place Q1353. Please update the mapping configuration (e.g., pvmap.csv) to resolve Delhi to Q9357528 and regenerate the golden observations.

"wikidataId/Q9357528"
References
  1. When mapping Indian administrative data at the AA1 level, use Q120971341 for Chandigarh, Q9357528 for Delhi, and Q200667 for Ladakh.

"rule_id": "check_deleted_records_percent",
"validator": "DELETED_RECORDS_PERCENT",
"params": {
"threshold": 0.1

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.

medium

The DELETED_RECORDS_PERCENT validator calculates the deletion percentage as a value out of 100 (i.e., (deleted_obs_count / previous_obs_count) * 100). Specifying 0.1 sets the threshold to 0.1% (one-tenth of a percent), which might be too strict and trigger false positives. If a 10% threshold was intended, this should be set to 10 or 10.0.

Suggested change
"threshold": 0.1
"threshold": 10

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.

1 participant