Skip to content

Introduce CustomErrors and DataErrors#554

Merged
gtrevisan merged 11 commits into
devfrom
glt/custom-errors
May 14, 2026
Merged

Introduce CustomErrors and DataErrors#554
gtrevisan merged 11 commits into
devfrom
glt/custom-errors

Conversation

@gtrevisan
Copy link
Copy Markdown
Member

  • revamp custom errors with a base CustomError:
    • add a generic DataError class:
      • add subclasses FetchDataError and NanDataError,
    • add a generic CalculationError class:
      • add subclass MismatchCalculationError,
  • have runner.py downgrade all CustomErrors to warnings,
  • have xr.py raise FetchDataErrors rather than catching KeyErrors,
  • add a required=True argument to all data getters to check for all-NaNs arrays,
  • apply a few of these custom exceptions.

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 introduces a structured custom-exception hierarchy for physics-method execution, updates data-connection getters to support required=True (all-NaN detection) and raise data-specific errors, and adjusts runners/physics methods to use and handle these new exceptions more consistently.

Changes:

  • Added CustomError base class with DataError/CalculationError branches and specific subclasses (e.g., FetchDataError, NanDataError, MismatchCalculationError).
  • Updated XarrayDataConnection and MDSConnection getters to accept required and raise FetchDataError/NanDataError where applicable.
  • Updated the physics-method runner to downgrade all CustomErrors to warnings and migrated several call sites to the new exception types.

Reviewed changes

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

Show a summary per file
File Description
disruption_py/machine/mast/util.py Uses MismatchCalculationError for interpolation input-length mismatch.
disruption_py/machine/mast/physics.py Uses MismatchCalculationError for density timebase length mismatch.
disruption_py/machine/d3d/physics.py Raises FetchDataError for an invalid data_source case.
disruption_py/machine/cmod/physics.py Adopts FetchDataError / MismatchCalculationError in a few data/mismatch scenarios and improves one error message.
disruption_py/inout/xr.py Adds required checks and raises FetchDataError/NanDataError instead of returning sentinels.
disruption_py/inout/mds.py Adds required checks and raises NanDataError for all-NaN data.
disruption_py/inout/base.py Extends the DataConnection interface with required: bool = False.
disruption_py/core/physics_method/runner.py Treats any CustomError as warning-level (vs. only CalculationError previously).
disruption_py/core/physics_method/errors.py Defines the new exception hierarchy and subclasses.

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

Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/cmod/physics.py
Comment thread disruption_py/core/physics_method/errors.py
@samc24
Copy link
Copy Markdown
Collaborator

samc24 commented May 13, 2026

mast/physics.py get_sxr (281/288) and get_dalpha (321/330) still have if X is not None: ... else: np.array([np.nan]) fallbacks for missing paths.

I think those else branches are unreachable after this PR since xr.get_data no longer returns None, it raises FetchDataError which runner.py catches and mocks the method's columns?

No functional change in the output ultimately .. so this is purely dead-code cleanup

Otherwise LGTM, can approve without change also

Copy link
Copy Markdown
Collaborator

@samc24 samc24 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gtrevisan gtrevisan removed the request for review from yumouwei May 14, 2026 14:02
@gtrevisan gtrevisan merged commit 7a8a38a into dev May 14, 2026
26 checks passed
@gtrevisan gtrevisan deleted the glt/custom-errors branch May 14, 2026 14:41
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.

4 participants