Skip to content

Small change to allow workflow to use convention-aware magnitudes without # ty: ignore - #19

Open
AndrewRidden-Harper wants to merge 1 commit into
mainfrom
support-BoldM-in-workflow
Open

Small change to allow workflow to use convention-aware magnitudes without # ty: ignore#19
AndrewRidden-Harper wants to merge 1 commit into
mainfrom
support-BoldM-in-workflow

Conversation

@AndrewRidden-Harper

Copy link
Copy Markdown

Small change to allow workflow to use convention-aware magnitudes without # ty: ignore.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the nshmdb library to use the BoldM class from source_modelling for parent fault magnitudes, updating type annotations, docstrings, and tests, while also pinning source-modelling>=2026.6.2 in requirements.txt. The feedback suggests using Mapping[str, Union[float, BoldM]] instead of dict[str, BoldM] for the parent_fault_magnitudes parameter to avoid static type-checking issues with invariant dictionaries, along with corresponding import and docstring updates.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread nshmdb/nshmdb.py

from nshmdb import query
from qcore import coordinates
from source_modelling.moment import BoldM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Import Mapping and Union from typing to support a more flexible type signature for parent_fault_magnitudes that accepts both standard float and BoldM values.

Suggested change
from source_modelling.moment import BoldM
from typing import Mapping, Union
from source_modelling.moment import BoldM

Comment thread nshmdb/nshmdb.py
Comment on lines 156 to 158
def most_likely_fault(
self, rupture_id: int, parent_fault_magnitudes: dict[str, float]
self, rupture_id: int, parent_fault_magnitudes: dict[str, BoldM]
) -> dict[str, float]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

By changing the type annotation of parent_fault_magnitudes from dict[str, float] to dict[str, BoldM], any existing or future callers passing a standard dict[str, float] will trigger static type-checking errors (since dict is invariant in Python).

To allow both float and BoldM (or other magnitude conventions) without type-checking issues, use Mapping[str, Union[float, BoldM]]. Since Mapping is covariant in its value type, this will seamlessly accept both dict[str, float] and dict[str, BoldM] without requiring any # type: ignore comments.

    def most_likely_fault(
        self,
        rupture_id: int,
        parent_fault_magnitudes: Mapping[str, Union[float, BoldM]],
    ) -> dict[str, float]:

Comment thread nshmdb/nshmdb.py
Comment on lines +178 to +181
parent_fault_magnitudes : dict[str, BoldM]
A mapping of parent fault names to their expected magnitudes (in the
`BoldM` convention). These magnitudes define the target values for
querying activity rates in the MFD table.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update the docstring to reflect that parent_fault_magnitudes can accept either standard float values or BoldM convention objects.

Suggested change
parent_fault_magnitudes : dict[str, BoldM]
A mapping of parent fault names to their expected magnitudes (in the
`BoldM` convention). These magnitudes define the target values for
querying activity rates in the MFD table.
parent_fault_magnitudes : Mapping[str, Union[float, BoldM]]
A mapping of parent fault names to their expected magnitudes (either as
floats or in the `BoldM` convention). These magnitudes define the target
values for querying activity rates in the MFD table.

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.

1 participant