Skip to content

Conversation

@mih
Copy link
Member

@mih mih commented Jul 16, 2025

No description provided.

mih added 6 commits July 16, 2025 10:54
In the DataLad world `CommandError` was traditionally a `RuntimeError`.
Whether or not that makes sense in the general case, one can have
different opinions about.

The rest of the world is using `subprocess.CalledProcessError`. This
exception type is very similar to `CommandError` in the properties
that it can capture. It is only missing the CWD info, and the
context message that comes with `RuntimeError` (see class docs).

This change connects the two worlds, by inheriting from both
`RuntimeError` and `CalledProcessError`, and using the properties
of both, where possible.

The behavior of `CommandError` remains unchanged, but it will now also
work with code that expects a `subprocess` exception.

Previously it was possible to not provide a return code when
instantiating the exception. This is not supported by
``CalledProcessError``. The behavior of ``CommandError`` of
not requiring a returncode is maintained (whether or not that makes
sense). In order to satisfy ``CalledProcessError`` as special
``CommandError.UNKNOWN_RETURNCODE`` value is introduced as the new
default. It is set to 32767, a value that should not occur in actual
use.
This helps to homogenize the rendered documentation.
This adopts the Google docstring conventions (without duplication of
type annotations in the docstring).

For now this is a trials (not amending the contributing guidelines yet).
@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.33%. Comparing base (324087f) to head (0ff629d).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
- Coverage   98.39%   98.33%   -0.07%     
==========================================
  Files          36       36              
  Lines        1435     1440       +5     
  Branches      130      131       +1     
==========================================
+ Hits         1412     1416       +4     
  Misses         12       12              
- Partials       11       12       +1     

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

@mih mih merged commit e82e5af into main Jul 16, 2025
9 checks passed
@mih mih deleted the error branch July 16, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants