Skip to content

Validate and JSON Serialize validate_types.Severity instances by name#1624

Merged
yarikoptic merged 3 commits intodandi:masterfrom
candleindark:serialize-severity
May 30, 2025
Merged

Validate and JSON Serialize validate_types.Severity instances by name#1624
yarikoptic merged 3 commits intodandi:masterfrom
candleindark:serialize-severity

Conversation

@candleindark
Copy link
Copy Markdown
Member

@candleindark candleindark commented May 7, 2025

This PR allows validation and JSON serialization of validate_types.Severity instances by name. It allows the represented data by validate_types.Severity to be more readily interpreted and generated by human users. This PR closes #1608.

Remaining TODOs:

@candleindark candleindark force-pushed the serialize-severity branch from 97aaf9a to 6cc665c Compare May 7, 2025 06:20
@codecov
Copy link
Copy Markdown

codecov bot commented May 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.90%. Comparing base (7707546) to head (b3854d4).
⚠️ Report is 120 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1624       +/-   ##
===========================================
+ Coverage   64.10%   88.90%   +24.79%     
===========================================
  Files          82       83        +1     
  Lines       11400    11505      +105     
===========================================
+ Hits         7308    10228     +2920     
+ Misses       4092     1277     -2815     
Flag Coverage Δ
unittests 88.90% <100.00%> (+24.79%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yarikoptic
Copy link
Copy Markdown
Member

yarikoptic commented May 7, 2025

please add a basic test for serialization of the ValidationResult into json and verify that Severity is literal, and that we can load from json back into ValidationResult.

@candleindark candleindark force-pushed the serialize-severity branch from 6cc665c to 597c303 Compare May 9, 2025 01:26
@candleindark candleindark added the patch Increment the patch version when merged label May 9, 2025
@candleindark candleindark force-pushed the serialize-severity branch 4 times, most recently from 8c439d2 to 9d61528 Compare May 12, 2025 02:25
@candleindark candleindark marked this pull request as ready for review May 12, 2025 02:39
@candleindark candleindark changed the title Serialize validate_types.Severity instances by their name Validate and JSON Serialize validate_types.Severity instances by name May 12, 2025
@candleindark candleindark force-pushed the serialize-severity branch 2 times, most recently from 16c7c63 to 7e7ef88 Compare May 19, 2025 22:06
@candleindark candleindark requested a review from Copilot May 19, 2025 22:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for validating Severity enum members by their name strings and ensuring JSON serialization outputs those names.

  • Introduces a dynamic _SeverityName enum to represent member names in JSON schema.
  • Implements _accept_severity_by_name and wraps Severity in an Annotated alias Severity_ with BeforeValidator and PlainSerializer.
  • Updates ValidationResult.severity to use Severity_ and extends tests accordingly.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
dandi/validate_types.py Define _SeverityName, _accept_severity_by_name, Severity_, and update field.
dandi/tests/test_validate_types.py Add tests covering serialization, round-trip, integer input, and invalid values.
Comments suppressed due to low confidence (1)

dandi/tests/test_validate_types.py:121

  • There are tests for integer and invalid inputs, but none explicitly verifying that a valid severity name string (like 'ERROR') is accepted and correctly mapped. Add a test case to ensure string names round-trip as intended.
@pytest.mark.parametrize("invalid_severity", ["foo", 42, True])

@candleindark candleindark marked this pull request as draft May 20, 2025 19:53
@yarikoptic
Copy link
Copy Markdown
Member

ready for review @candleindark ?

@candleindark candleindark marked this pull request as ready for review May 28, 2025 16:41
…ity` instances by name

This allows the data to be handled by
human more readily
…sult.origin_result`

Doing this eliminates false warnings in PyCharm
@candleindark candleindark requested a review from yarikoptic May 28, 2025 19:31
@candleindark
Copy link
Copy Markdown
Member Author

ready for review @candleindark ?

Yes. It is ready now.

@yarikoptic yarikoptic merged commit bbd1670 into dandi:master May 30, 2025
28 checks passed
@github-actions
Copy link
Copy Markdown

🚀 PR was released in 0.69.2 🚀

@candleindark candleindark deleted the serialize-severity branch June 2, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Increment the patch version when merged released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make values of Severity serializable to meaningful strings

3 participants