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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
88b7ac5
Merge pull request #251 from NASA-IMPACT/dev
jenny-m-wood Aug 2, 2023
6195cea
Merge pull request #268 from NASA-IMPACT/dev
jenny-m-wood Oct 11, 2023
bed9e24
Merge pull request #271 from NASA-IMPACT/dev
jenny-m-wood Dec 5, 2023
e800dde
Merge pull request #275 from NASA-IMPACT/dev
jenny-m-wood Mar 1, 2024
d8c6b5c
Merge pull request #276 from NASA-IMPACT/dev
jenny-m-wood Mar 1, 2024
05191f1
worked on the standard product check error
binni979 Apr 29, 2024
5d73536
Added a docstrings in standard_product_check function
binni979 Apr 29, 2024
42d575b
Modified standard product check function
binni979 May 6, 2024
f2287ea
worked on the one_time_presence_check function
binni979 May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion pyQuARC/code/custom_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def one_item_presence_check(*field_values):
break

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

@staticmethod
def dif_standard_product_check(*field_values):
"""
Expand All @@ -119,6 +119,22 @@ def dif_standard_product_check(*field_values):
break
return {"valid": validity, "value": value}

@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.

This function iterates over a tuple of field values. It checks each field value to determine
if it contains the field_value True or False.
"""
validity = False
value = None

for field_value in field_values:
if field_value in [True, False]:
value = field_value
validity = True
break
return {"valid": validity, "value": value}

@staticmethod
def license_url_description_check(description_field, url_field, license_text):
"""
Expand Down
5 changes: 5 additions & 0 deletions pyQuARC/schemas/checks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@
"check_function": "dif_standard_product_check",
"available": true
},
"standard_product_check": {
"data_type": "custom",
"check_function": "standard_product_check",
"available": true
},
"doi_link_update": {
"data_type": "url",
"check_function": "doi_link_update",
Expand Down
2 changes: 1 addition & 1 deletion pyQuARC/schemas/rule_mapping.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2711,7 +2711,7 @@
]
},
"severity": "warning",
"check_id": "one_item_presence_check"
"check_id": "standard_product_check"
},
"dif_standard_product_check": {
"rule_name": "Standard Product Check",
Expand Down
Loading