Skip to content

Remove redundant asserts #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lispandfound wants to merge 1 commit into
base: pegasus
Choose a base branch
from
Open

Remove redundant asserts #95

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions workflow/scripts/gcmt_to_realisation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,6 @@ def gcmt_to_realisation(
strike2 = gcmt_solutions.at[gcmt_event_id, "strike2"]
dip2 = gcmt_solutions.at[gcmt_event_id, "dip2"]
rake2 = gcmt_solutions.at[gcmt_event_id, "rake2"]
assert isinstance(strike1, float | int)
assert isinstance(dip1, float | int)
assert isinstance(rake1, float | int)
assert isinstance(strike2, float | int)
assert isinstance(dip2, float | int)
assert isinstance(rake2, float | int)
nodal_plane_1 = NodalPlane(strike1, dip1, rake1)
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This PR removes type asserts for strike/dip/rake because pandas returns np.float64, but there are still asserts later in this function (assert isinstance(magnitude, float) and assert isinstance(centroid_depth, float)) that will also fail for np.float64 (and potentially int). Consider removing these asserts too, or replacing them with a numbers.Real/np.floating check or explicit float(...) conversion at the point of use.

Copilot uses AI. Check for mistakes.
nodal_plane_2 = NodalPlane(strike2, rake2, dip2)
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

nodal_plane_2 is constructed with arguments in a different order than nodal_plane_1 (and likely the NodalPlane API). Here it passes (strike2, rake2, dip2), which swaps dip and rake and will produce an incorrect nodal plane; use the same (strike, dip, rake) ordering as for plane 1 (and as implied by NodalPlane(**solution["nodalPlanes"][1])).

Suggested change
nodal_plane_2 = NodalPlane(strike2, rake2, dip2)
nodal_plane_2 = NodalPlane(strike2, dip2, rake2)

Copilot uses AI. Check for mistakes.
elif gcmt_event_id in automated_gcmt_solutions:
Expand Down
Loading