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

Standard product check #285

Merged
merged 9 commits into from
May 13, 2024
Merged

Standard product check #285

merged 9 commits into from
May 13, 2024

Conversation

binni979
Copy link
Contributor

@binni979 binni979 commented Apr 29, 2024

This is the pull request to solve this issue.

Overview:
In the pyQuarc, while running the collection, it showed the warning message like this:
Screenshot 2024-04-29 at 9 57 31 AM

Changes made:
Added a standard_product_check function in the custom_validator file according to the rule mapping.json and check message.json. Both JSON uses the standard_product_check function but the function was not created.
Now, no error is shown.
Tested on this ' C1577484501-LARC_ASDC' (umm-c) collection to make sure the if the warning shows up.

jenny-m-wood and others added 7 commits August 2, 2023 15:13
Updates: Adding Checks and Resolving Inconsistencies
Minor fixes and additional checks:
- Updated UMM-C schema file
- Added science_keywords_presence_check
- Added DOI authority presence check for echo-c and umm-c
- Adjusted output message for url_check
- Added orbit fields to rule_mapping for spatial_extent_fulfillment_check
- Resolved ISO standard type in check_messages
Updates to README and umm-g schema file
pyQuARC DOI creation updates
Created new version for citation file and README updates
@binni979 binni979 marked this pull request as ready for review April 29, 2024 15:00
@binni979 binni979 requested review from slesaad and xhagrg April 29, 2024 15:01
@binni979 binni979 self-assigned this Apr 29, 2024
Comment on lines 122 to 124
@staticmethod
def standard_product_check(*field_values):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Why rename the method? What's the benefit of using one or the other method name? Can the older model name not be used?

Copy link
Contributor

Choose a reason for hiding this comment

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

we already have a dif_standard_product_check method, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we have dif_standard_product_check that is used to check the extended metadata for DIF schema, but the standard_product_check is used to check the StandardProduct key inside the collection for schema echo-c and umm-c.
I thought, creating a new function solved the problem, but it shows the error again and now I changed the code which is different from the previous function dif_standard_product_check .

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's just checking values in a constrained list, don't we already have a check that does this? why start a new one?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like Standard Product Check is using the already existing one_item_presence_check check, which goes like this:

def one_item_presence_check(*field_values):
        """
        Checks if one of the specified fields is populated
        At least one of the `field_values` should not be null
        It is basically a OneOf check
        """
        validity = False
        value = None

        for field_value in field_values:
            if field_value:
                value = field_value
                validity = True
                break

        return {"valid": validity, "value": value}

This check is failing for the collection mentioned in the issue because the value is False and the if field_value: clause fails. It can be remedied by doing something like this instead:

if field_value is not None:

@binni979

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @slesaad didi. Worked on that.

Copy link
Member

Choose a reason for hiding this comment

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

Did you test it? Everything working well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested it on the "UMM-C record (C1577484501-LARC_ASDC)". Yes, everything working well.

@slesaad slesaad merged commit ec915ef into dev May 13, 2024
1 check passed
@slesaad slesaad mentioned this pull request Jun 24, 2024
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.

4 participants