Refactor and cleanup restartthinner #847

larsevj · 2026-01-16T15:02:36Z

No description provided.

codecov-commenter · 2026-01-16T15:13:09Z

Codecov Report

❌ Patch coverage is 98.11321% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.52%. Comparing base (4ac49d5) to head (eb3e265).

Files with missing lines	Patch %	Lines
src/subscript/restartthinner/restartthinner.py	98.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
+ Coverage   83.44%   83.52%   +0.07%     
==========================================
  Files          49       49              
  Lines        7287     7279       -8     
==========================================
- Hits         6081     6080       -1     
+ Misses       1206     1199       -7

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

Copilot

Pull request overview

This PR refactors and modernizes the restartthinner module by improving code quality, replacing deprecated patterns, and enhancing maintainability.

Changes:

Replaced os.system() calls with subprocess.run() for better error handling and portability
Refactored find_resdata_app() to use shutil.which() instead of manual PATH traversal
Simplified date_slicer() to return a list directly instead of a dict, improving clarity

Comments suppressed due to low confidence (1)

src/subscript/restartthinner/restartthinner.py:69

There are two spelling errors in this comment: "dont't" should be "don't" and "remainding" should be "remaining".

    First unpacking a UNRST file, then deleting dates the dont't want, then
    pack the remainding files into a new UNRST file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/subscript/restartthinner/restartthinner.py

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/subscript/restartthinner/restartthinner.py

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/subscript/restartthinner/restartthinner.py

tests/test_restartthinner.py

src/subscript/restartthinner/restartthinner.py

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/subscript/restartthinner/restartthinner.py

Copilot · 2026-01-19T11:20:14Z

src/subscript/restartthinner/restartthinner.py

    if numberofslices > 1:
-        slicedates = pandas.DatetimeIndex(
-            numpy.linspace(
-                pandas.Timestamp(restart_dates[0]).value,
-                pandas.Timestamp(restart_dates[-1]).value,
+        slicedates = pd.DatetimeIndex(
+            np.linspace(
+                pd.Timestamp(restart_dates[0]).value,
+                pd.Timestamp(restart_dates[-1]).value,
                int(numberofslices),
            )
        ).to_list()
    else:
        slicedates = [restart_dates[-1]]  # Only return last date if only one is wanted

-    slicerstindices = list(
-        date_slicer(slicedates, restart_dates, restart_indices).values()
-    )
-    slicerstindices.sort()
-    slicerstindices = list(set(slicerstindices))  # uniquify
+    slicerstindices = date_slicer(slicedates, restart_dates, restart_indices)
+    slicerstindices = sorted(set(slicerstindices))  # uniquify


When numberofslices equals the number of existing restart points, the date_slicer logic may still try to calculate evenly-spaced dates which could theoretically select duplicates. The sorted(set(...)) on line 179 handles this, but it means the user might get fewer restart points than requested. Consider adding a check or warning when numberofslices >= len(restart_indices) to inform the user that all restart points are being kept.

src/subscript/restartthinner/restartthinner.py

tests/test_restartthinner.py

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/subscript/restartthinner/restartthinner.py

Copilot · 2026-02-05T12:15:34Z

tests/test_restartthinner.py

+        restartthinner.main()
+
+    # Check that log output is empty
+    assert len(caplog.text) == 0


Use assert not caplog.text instead of assert len(caplog.text) == 0 for a more Pythonic check of empty strings.

Suggested change

assert len(caplog.text) == 0

assert not caplog.text

larsevj requested a review from Copilot January 19, 2026 09:01

larsevj marked this pull request as ready for review January 19, 2026 09:01

Copilot started reviewing on behalf of larsevj January 19, 2026 09:01 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

src/subscript/restartthinner/restartthinner.py Show resolved Hide resolved

src/subscript/restartthinner/restartthinner.py Outdated Show resolved Hide resolved

src/subscript/restartthinner/restartthinner.py Outdated Show resolved Hide resolved

larsevj force-pushed the simplify_restartthinner branch from 56537b7 to 1b28dbe Compare January 19, 2026 09:36

larsevj requested review from Copilot January 19, 2026 09:51

Copilot started reviewing on behalf of larsevj January 19, 2026 09:51 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

larsevj force-pushed the simplify_restartthinner branch from 1b28dbe to 925b484 Compare January 19, 2026 10:06

larsevj requested a review from Copilot January 19, 2026 10:08

Copilot started reviewing on behalf of larsevj January 19, 2026 10:08 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

larsevj force-pushed the simplify_restartthinner branch from 925b484 to 633967e Compare January 19, 2026 11:05

larsevj requested a review from Copilot January 19, 2026 11:13

Copilot started reviewing on behalf of larsevj January 19, 2026 11:14 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

larsevj force-pushed the simplify_restartthinner branch 2 times, most recently from 73c3fa3 to c172ddf Compare January 26, 2026 10:09

larsevj requested a review from Copilot January 26, 2026 10:10

Copilot AI reviewed Jan 26, 2026

View reviewed changes

larsevj requested a review from alifbe January 26, 2026 10:11

Refactor and cleanup restartthinner

eb3e265

larsevj force-pushed the simplify_restartthinner branch from c172ddf to eb3e265 Compare February 5, 2026 11:54

larsevj requested a review from Copilot February 5, 2026 12:14

Copilot AI reviewed Feb 5, 2026

View reviewed changes

larsevj requested review from alifbe and removed request for alifbe February 5, 2026 12:16

larsevj requested a review from rnyb February 5, 2026 12:16

rnyb approved these changes Feb 7, 2026

View reviewed changes

larsevj merged commit e949b0d into main Feb 9, 2026
9 checks passed

larsevj deleted the simplify_restartthinner branch February 9, 2026 08:24

Refactor and cleanup restartthinner #847

Refactor and cleanup restartthinner #847

Uh oh!

Conversation

larsevj commented Jan 16, 2026

Uh oh!

codecov-commenter commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

larsevj Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Jan 16, 2026 •

edited

Loading