⛈️ #15
⛈️ #15
Conversation
Reviewer's Guide by SourceryThis pull request introduces significant changes to the File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leaver2000 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 13 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 5 issues found
- 🟡 Complexity: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| x, y = self.x, self.y | ||
| nx = np.arange(x.shape[0]) | ||
| if which == "bottom": | ||
| idx = np.argmin(~np.isnan(x), axis=1) - 1 # the last non-nan value |
There was a problem hiding this comment.
issue (bug_risk): Potential off-by-one error
The calculation np.argmin(~np.isnan(x), axis=1) - 1 might lead to an off-by-one error if all values are NaN. Consider adding a check to handle this case.
| *args: _P.args, | ||
| **kwargs: _P.kwargs, | ||
| ) -> _T: | ||
| # TODO |
There was a problem hiding this comment.
issue: Incomplete TODO comment
The TODO comment is incomplete. Consider providing more details or removing it if it is no longer relevant.
| # TODO: because the _multiple_el_lfc_options function is highly dependent on the output of | ||
| # find_intersections combining them into a class is probably the best idea. |
There was a problem hiding this comment.
issue: Incomplete TODO comment
The TODO comment suggests combining functions into a class but does not provide enough context. Consider elaborating on the plan or removing the comment if it is no longer relevant.
| if mixed_layer_depth is None: | ||
| r = mixing_ratio(uf.saturation_vapor_pressure(td0[:, newaxis]), p0[:, newaxis]) | ||
| else: |
There was a problem hiding this comment.
issue: Unimplemented feature
The else block raises a NotImplementedError for mixed_layer_depth. Consider implementing this feature or providing more context on when it will be implemented.
|
|
||
|
|
||
| @overload | ||
| def exactly_2d( |
There was a problem hiding this comment.
suggestion: Function name clarity
Consider renaming the function exactly_2d to something more descriptive, such as ensure_2d, to better convey its purpose.
| def exactly_2d( | |
| @overload | |
| def ensure_2d( | |
| __x: np.ndarray[Any, np.dtype[np.float_]], | |
| ) -> np.ndarray[shape[N, Z], np.dtype[np.float_]]: ... |
| t0 = temperature[:, 0] # (N,) | ||
| dewpoint_start: np.ndarray[shape[N], np.dtype[float_]] | None = None, | ||
| ) -> ElementNd[shape[N], float_]: | ||
| p0, t0 = pressure[:, 0], temperature[:, 0] | ||
| if dewpoint_start is None: |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Replace if statement with if expression (
assign-if-exp) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| dcape = -(Rd * F.nantrapz(delta, logp, axis=1)) | ||
|
|
||
| return dcape |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| dcape = -(Rd * F.nantrapz(delta, logp, axis=1)) | |
| return dcape | |
| return -(Rd * F.nantrapz(delta, logp, axis=1)) |
| if len(values) == 1: | ||
| return values[0] | ||
|
|
||
| return tuple(values) |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if len(values) == 1: | |
| return values[0] | |
| return tuple(values) | |
| return values[0] if len(values) == 1 else tuple(values) |
| values = [] | ||
| for x in args: | ||
| values.append(np.where(np.isnan(x), np.roll(x, 1, axis=1), x)) | ||
|
|
||
| if len(values) == 1: | ||
| return values[0] | ||
|
|
||
| def interp_nan(x: NDArray[float_], /, copy: bool = True) -> NDArray[float_]: | ||
| if copy is True: | ||
| x = x.copy() | ||
| mask = np.isnan(x) | ||
| x[mask] = np.interp(np.flatnonzero(mask), np.flatnonzero(~mask), x[~mask]) | ||
| return x | ||
| return tuple(values) |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Convert for loop into list comprehension (
list-comprehension) - Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| values = [] | |
| for x in args: | |
| values.append(np.where(np.isnan(x), np.roll(x, 1, axis=1), x)) | |
| if len(values) == 1: | |
| return values[0] | |
| def interp_nan(x: NDArray[float_], /, copy: bool = True) -> NDArray[float_]: | |
| if copy is True: | |
| x = x.copy() | |
| mask = np.isnan(x) | |
| x[mask] = np.interp(np.flatnonzero(mask), np.flatnonzero(~mask), x[~mask]) | |
| return x | |
| return tuple(values) | |
| values = [np.where(np.isnan(x), np.roll(x, 1, axis=1), x) for x in args] | |
| return values[0] if len(values) == 1 else tuple(values) |
| equal_nan: bool = False, | ||
| ) -> NDArray[np.bool_]: | ||
| return np.logical_and(op(a, b), np.isclose(a, b, rtol=rtol, atol=atol, equal_nan=equal_nan)) | ||
| if log_x is True: |
There was a problem hiding this comment.
suggestion (code-quality): Simplify comparison to boolean (simplify-boolean-comparison)
| if log_x is True: | |
| if log_x: |
Summary by Sourcery
This pull request introduces several new features, including the
broadcast_nzdecorator, new element classes, and functions for calculating equilibrium levels, downdraft CAPE, and most unstable parcel properties. It also includes bug fixes, enhancements to existing functions, and updates to the documentation and test cases.broadcast_nzdecorator to handle broadcasting of input arrays for various functions.ElementNd,Element1d, andElement2dto represent elements with generic types and dimensions.el_lfc,downdraft_cape, andmost_unstable_parcelto calculate equilibrium levels, downdraft CAPE, and most unstable parcel properties respectively.dewpoint_from_specific_humidityfunction in_ufunc.pyx.find_intersectionsfunction to useElement2dclass for better structure and readability.intersect_1dfunction infunctional.cppto handle both increasing and decreasing directions and added support for logarithmic x-values.parcel_profileandparcel_profile_with_lclfunctions to handle both broadcast and matrix modes more efficiently.README.mdwith detailed type annotations and usage examples for better clarity.README.mdto include detailed type annotations and usage examples for theparcel_profilefunction.functional_test.pyandcore_test.pyto validate the new features and enhancements.find_intersectionsfunction andElement2dclass.