Skip to content

Use natsort to sort the produced csv rows from gather_summary_observations#13684

Merged
SAKavli merged 1 commit into
equinor:mainfrom
SAKavli:natsort-csv-columns
Jun 4, 2026
Merged

Use natsort to sort the produced csv rows from gather_summary_observations#13684
SAKavli merged 1 commit into
equinor:mainfrom
SAKavli:natsort-csv-columns

Conversation

@SAKavli
Copy link
Copy Markdown
Contributor

@SAKavli SAKavli commented Jun 2, 2026

Issue
Resolves #13665

Approach
I believe this is ready to go unless I have misunderstood the prioritization of the sorting.

The code natsorts on the complete string, but perhaps it is more sensible to sort based on well names first, then summary keyword?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.34%. Comparing base (3fb9cab) to head (44ca2e7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13684      +/-   ##
==========================================
- Coverage   89.39%   89.34%   -0.05%     
==========================================
  Files         469      469              
  Lines       33406    33420      +14     
==========================================
- Hits        29862    29859       -3     
- Misses       3544     3561      +17     
Flag Coverage Δ
cli-tests 35.78% <100.00%> (-0.05%) ⬇️
fuzz 43.37% <33.33%> (-0.05%) ⬇️
gui-tests 59.25% <33.33%> (-0.06%) ⬇️
performance-and-unit-tests 77.79% <100.00%> (-0.05%) ⬇️
test 44.91% <0.00%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
src/ert/gather_summary_observations.py 79.66% <100.00%> (+0.08%) ⬆️

... and 6 files with indirect coverage changes

@SAKavli SAKavli force-pushed the natsort-csv-columns branch 2 times, most recently from c48332c to 517e58c Compare June 2, 2026 13:33
@SAKavli SAKavli added the release-notes:unreleased-feature-changes PR with changes to a feature which is not yet released. Not for introduction of new features! label Jun 2, 2026
@SAKavli SAKavli changed the title Natsort csv columns Use natsort to sort the produced csv rows from gather_summary_observations Jun 2, 2026
@SAKavli SAKavli requested a review from Copilot June 2, 2026 13:51
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

This PR updates convert_summary_observations (used by the gather_summary_observations CLI flow) to output CSV rows in a natural sort order (e.g., OP2, OP4, OP10, OP30) rather than preserving dict insertion order, addressing the ordering issue described in #13665.

Changes:

  • Sort CSV output rows by summary key using natsort.natsorted(...).
  • Add a unit test that asserts natural ordering of generated CSV rows for well keys containing numbers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ert/gather_summary_observations.py Iterate summary keys using natsorted(...) to produce naturally ordered CSV rows.
tests/ert/unit_tests/cli/test_gather_summary_observations.py Add regression test verifying natural sort order in the produced CSV.

Comment thread src/ert/gather_summary_observations.py
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 2, 2026

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing SAKavli:natsort-csv-columns (44ca2e7) with main (9643008)

Open in CodSpeed

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

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

Comment thread tests/ert/unit_tests/cli/test_gather_summary_observations.py
Comment thread tests/ert/unit_tests/cli/test_gather_summary_observations.py
Natsort is a transitive dependency of resdata, and should be an
explicit dependency of ert when used in Ert code.
@SAKavli SAKavli force-pushed the natsort-csv-columns branch from 2618c13 to 44ca2e7 Compare June 3, 2026 07:45
@SAKavli SAKavli merged commit 4b035d5 into equinor:main Jun 4, 2026
118 of 121 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:unreleased-feature-changes PR with changes to a feature which is not yet released. Not for introduction of new features!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gather_summary_observations should The csv rows should be ordered by the original keyword (eg. WOPR:OP1) using natsort

4 participants