Skip to content
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.

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

gh-480: fix mypy failures #490

Merged
merged 3 commits into from
Feb 1, 2025
Merged

gh-480: fix mypy failures #490

merged 3 commits into from
Feb 1, 2025

Conversation

Saransh-cpp
Copy link
Member

Description

Fixes most of the errors. See my comments below for a better explanation.

Refs: #480

Changelog entry

Checks

  • Is your code passing linting?
  • Is your code passing tests?
  • Have you added additional tests (if required)?
  • Have you modified/extended the documentation (if required)?
  • Have you added a one-liner changelog entry above (if required)?

@@ -166,7 +166,7 @@ def trapezoid_product(
y = np.interp(x, *f)
for f_ in ff:
y *= np.interp(x, *f_)
return np.trapezoid(y, x, axis=axis) # type: ignore[return-value]
return np.trapezoid(y, x, axis=axis)
Copy link
Member Author

Choose a reason for hiding this comment

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

The failing ignore[return-value] checks were on numpy 2.2.0, and should be resolved now.

@@ -52,7 +52,7 @@ def effective_bias(
z: npt.NDArray[np.float64],
bz: npt.NDArray[np.float64],
w: RadialWindow,
) -> npt.NDArray[np.float64]:
) -> float | npt.NDArray[np.double]:
Copy link
Member Author

Choose a reason for hiding this comment

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

  • np.trapezoid can return a float
  • Division between np.float64 and np.float64 can result in np.double ("float-any") for some reason.

@@ -742,7 +742,7 @@ def _uniform_grid(
if step is not None and num is None:
return np.arange(start, np.nextafter(stop + step, stop), step)
if step is None and num is not None:
return np.linspace(start, stop, num + 1) # type: ignore[return-value]
return np.linspace(start, stop, num + 1, dtype=np.float64)
Copy link
Member Author

Choose a reason for hiding this comment

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

Returns "float-any" so be explicit about the type

z = np.linspace(zmin, zmax, n)
w = wht(z) # type: ignore[arg-type]
z = np.linspace(zmin, zmax, n, dtype=np.float64)
w = wht(z)
zeff = np.trapezoid(w * z, z) / np.trapezoid(w, z)
Copy link
Member Author

Choose a reason for hiding this comment

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

np.trapezoid can return an array and I don't think they have enough overloads in place -- therefore zeff can be an array and the type signature of RadialWindow should reflect this

@@ -442,7 +442,7 @@ def add_window(self, delta: npt.NDArray[np.float64], w: RadialWindow) -> None:
def add_plane(
self,
delta: npt.NDArray[np.float64],
zsrc: float,
zsrc: float | npt.NDArray[np.float64],
Copy link
Member Author

Choose a reason for hiding this comment

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

If zeff can be an array, zsrc in add_window can be one too, and add_window passes zsrc to add_plane

Comment on lines +413 to +414
self.z2: float | npt.NDArray[np.float64] = 0.0
self.z3: float | npt.NDArray[np.float64] = 0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

Because of zsrc's new type signature

@Saransh-cpp Saransh-cpp mentioned this pull request Jan 28, 2025
@ntessore
Copy link
Collaborator

ntessore commented Feb 1, 2025

Merging this to unblock other PRs' tests.

@ntessore ntessore merged commit 17dfffd into main Feb 1, 2025
16 checks passed
@ntessore ntessore deleted the saransh/fix-mypy branch February 1, 2025 16:11
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