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

validate tagged objects using schema for tag #1737

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Jan 19, 2024

Passing a Tagged object to a validator with a custom schema fails to validate the Tagged object using the schema associated with the tag.

This bug is impacting roman_datamodels (which uses this pattern for assignment validation). Without this PR the following will not raise a ValidationError:

import roman_datamodels.maker_utils

m = roman_datamodels.datamodels.ImageModel(roman_datamodels.maker_utils.mk_level2_image())
inst = m.meta.instrument
inst['name'] = 'a'
m.meta.instrument = inst

With this PR a ValidationError is raised ('a' is not one of ['WFI']).

This PR adds a test for that issue and fixes it by shuffling around a few items in the validator (so that tags trigger schema lookups even if a custom schema was provided to the validator).

This shuffling had the side effect of changing the validation error for one test (test_history) to what looks like a more helpful message.

This PR includes some changes from #1729 which should be merged first.

Running roman regtests as this will change how validation on assignment behaves: https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/605/

Checklist:

  • pre-commit checks ran successfully
  • tests ran successfully
  • for a public change, a changelog entry was added
  • for a public change, documentation was updated
  • for any new features, unit tests were added

@braingram braingram force-pushed the validate_tagged branch 2 times, most recently from 583b35e to b5d21f1 Compare January 19, 2024 20:57
@braingram braingram marked this pull request as ready for review March 28, 2024 16:43
@braingram braingram requested a review from a team as a code owner March 28, 2024 16:43
@braingram
Copy link
Contributor Author

The roman_datamodels downstream job is now failing with this PR with a 143 exit code.

Running these locally they pass but use a considerable amount of RAM (>8GB).

Running the same tests with the released asdf takes ~600MB.

This was only seen in recent runs so something has changed in roman_datamodels that now conflicts with the changes in this PR. I'm going to convert this to draft until the cause of the memory issue can be sorted.

@braingram braingram marked this pull request as draft May 10, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants