Skip to content

Remove pydantic setting use_enum_values=True#12464

Merged
berland merged 1 commit intoequinor:mainfrom
berland:no_use_enum_values
Dec 18, 2025
Merged

Remove pydantic setting use_enum_values=True#12464
berland merged 1 commit intoequinor:mainfrom
berland:no_use_enum_values

Conversation

@berland
Copy link
Contributor

@berland berland commented Dec 5, 2025

Reverting back to pydantic default behaviour. This implies at least that the queue_system member of QueueConfig is always of type QueueSystem, and never just a str, which breaks what the type hints say.

Issue
Related to #12462

Approach
Modify ruamel instead.

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@berland berland self-assigned this Dec 5, 2025
@berland berland added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Dec 5, 2025
@berland berland added this to SCOUT Dec 5, 2025
@berland berland moved this to Ready for Review in SCOUT Dec 5, 2025
@berland
Copy link
Contributor Author

berland commented Dec 5, 2025

Does not fly as Everest is using ruamel which doesn't like StrEnum. Fixed

@berland berland moved this from Ready for Review to In Progress in SCOUT Dec 5, 2025
@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.58%. Comparing base (4fb794e) to head (c31a1a0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12464      +/-   ##
==========================================
- Coverage   90.60%   90.58%   -0.02%     
==========================================
  Files         433      433              
  Lines       29486    29492       +6     
==========================================
  Hits        26716    26716              
- Misses       2770     2776       +6     
Flag Coverage Δ
cli-tests 37.62% <ø> (ø)
gui-tests 69.04% <ø> (-0.03%) ⬇️
performance-and-unit-tests 74.18% <50.00%> (-0.01%) ⬇️
test 38.48% <100.00%> (+0.01%) ⬆️

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.

@berland berland force-pushed the no_use_enum_values branch 2 times, most recently from 9524851 to c81802e Compare December 5, 2025 12:08
@berland berland requested a review from Copilot December 5, 2025 12:08
@berland berland force-pushed the no_use_enum_values branch from c81802e to 22456f8 Compare December 5, 2025 12:11
Copy link

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

This PR removes the pydantic use_enum_values=True configuration setting, reverting to pydantic's default behavior where enum fields are represented as enum instances rather than their string values. This change ensures that type hints accurately reflect runtime behavior, particularly for the QueueConfig.queue_system field which should be a QueueSystem enum instance, not a string.

Key Changes:

  • Removed use_enum_values=True from QueueOptions base model configuration
  • Added StrEnum representer to handle YAML serialization of enum instances in Everest config dumping

Reviewed changes

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

File Description
src/ert/config/queue_config.py Removes use_enum_values=True from QueueOptions model config, ensuring enum fields remain as enum instances
src/everest/config/everest_config.py Adds StrEnum representer for YAML serialization and necessary imports to properly serialize enum instances when dumping config to YAML

@berland berland moved this from In Progress to Ready for Review in SCOUT Dec 5, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 5, 2025

CodSpeed Performance Report

Merging #12464 will not alter performance

Comparing berland:no_use_enum_values (c31a1a0) with main (4fb794e)

Summary

✅ 22 untouched

@berland berland moved this from Ready for Review to In Progress in SCOUT Dec 5, 2025
@berland berland force-pushed the no_use_enum_values branch 2 times, most recently from d931010 to 9e79a35 Compare December 5, 2025 14:10
@berland
Copy link
Contributor Author

berland commented Dec 5, 2025

Todo: Check all yaml.dump in the Everest codebase and make it into a common function.

Reverting back to pydantic default behaviour. This implies
at least that the queue_system member of QueueConfig is always
of type QueueSystem, and never just a `str`, which breaks what
the type hints say.
@berland berland force-pushed the no_use_enum_values branch from a2ad5f3 to c31a1a0 Compare December 9, 2025 14:32
@berland berland moved this from In Progress to Ready for Review in SCOUT Dec 9, 2025
@andreas-el
Copy link
Contributor

Todo: Check all yaml.dump in the Everest codebase and make it into a common function.

Will this be verification for this PR, or a separate issue?

Copy link
Contributor

@andreas-el andreas-el left a comment

Choose a reason for hiding this comment

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

👌

@github-project-automation github-project-automation bot moved this from Ready for Review to Reviewed in SCOUT Dec 10, 2025
@berland
Copy link
Contributor Author

berland commented Dec 18, 2025

Todo: Check all yaml.dump in the Everest codebase and make it into a common function.

Will this be verification for this PR, or a separate issue?

This has now been fixed in this PR.

@berland berland merged commit 9aad2b7 into equinor:main Dec 18, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from Reviewed to Done in SCOUT Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:maintenance Automatically categorise as maintenance change in release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants