Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return correct result when IDs match but types differ #50

Closed
jpmckinney opened this issue Nov 11, 2021 · 3 comments
Closed

Return correct result when IDs match but types differ #50

jpmckinney opened this issue Nov 11, 2021 · 3 comments
Labels
bug Something isn't working compiled release checks Relating to compiled release-level checks dataset checks Relating to dataset-level checks
Milestone

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Nov 11, 2021

From docs:

Some ID fields allow both integer and string values. When resolving references by comparing IDs, the check should fail if the IDs are different types. It should neither succeed nor N/A (it is likely to N/A if IDs are not coerced to string).

For example, awards_status should fail with this fixture. The data currently passes because the publisher was "lucky" enough to use different types for the id and awardID.

        (
            {"awards": [{"status": "pending", "id": 1}], "contracts": [{"awardID": "1"}]},
            {"processed_awards": [{"path": "awards[0]", "id": 0, "result": False}]},
            1,
            0,
        ),
@jpmckinney jpmckinney added compiled release checks Relating to compiled release-level checks dataset checks Relating to dataset-level checks labels Nov 11, 2021
@jpmckinney jpmckinney added this to the Cleanup milestone Jan 17, 2023
@jpmckinney jpmckinney added the bug Something isn't working label Jan 17, 2023
@jpmckinney jpmckinney modified the milestones: Cleanup, Priority Jan 17, 2023
@jpmckinney
Copy link
Member Author

jpmckinney commented Jan 19, 2023

Revisiting this issue:

The "main" reference check should fail if types mismatch (but it can have a useful message in the reason). Related checks should do type coercion to identify errors that might be "shadowed" by the type mismatch.

  • reference.parties: Add different message if coerced id values match, but types don't match.

Dependent checks

  • consistent.org_ref_name
  • consistent.parties_role
  • consistent.roles

jpmckinney added a commit that referenced this issue Jan 19, 2023
…stent.roles, #50

- consistent.org_ref_name: Don't check whether id and name are non-empty (OK for reference to match on "", etc.).
- consistent.roles: Don't check whether id is non-null (OK for reference to match on null).

Note: If later restored, use `is not None` for id, since id can be set to 0.
jpmckinney added a commit that referenced this issue Jan 19, 2023
…milestones_dates). perf: Call get_values() outside the nested for-loop.

- coherent.dates: Do type casting to match id and awardID #50
- coherent.dates: Don't check whether id and awardID are non-blank (OK for reference to match on "", etc.)
@jpmckinney
Copy link
Member Author

jpmckinney commented Jan 19, 2023

Main check

  • reference.contract_in_awards

Dependent checks: Add different message if coerced id values match, but types don't match.

  • coherent.awards_status
  • coherent.dates
  • consistent.contracts_value

jpmckinney added a commit that referenced this issue Jan 19, 2023
…ing to match id and awardID in coherent.awards_status #50
@jpmckinney
Copy link
Member Author

I think I got them all now. I just did regex checks.

I think the dataset-level and time-based checks compare ocid, which should always be a string due to the prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiled release checks Relating to compiled release-level checks dataset checks Relating to dataset-level checks
Projects
None yet
Development

No branches or pull requests

1 participant