🔢pressure and height vectors #25

leaver2000 · 2024-07-04T10:55:05Z

Summary by Sourcery

This pull request introduces new vertical_vector, pressure_vector, and height_vector classes to enhance atmospheric pressure and height calculations. It also refactors existing functions to use these new classes, improving code organization and readability. Additionally, new tests are added to ensure the correctness of these changes.

New Features:
- Introduced vertical_vector class as an abstract base class for pressure_vector and height_vector with methods for comparison and conversion.
- Added pressure_vector and height_vector classes inheriting from vertical_vector, providing methods for atmospheric pressure and height calculations.
- Implemented ncape function for calculating normalized convective available potential energy.
Enhancements:
- Refactored pressure_vector and height_vector to use new vertical_vector base class, improving code organization and reusability.
- Updated bunkers_storm_motion and other functions to use new pressure_vector and height_vector methods for cleaner and more readable code.
- Revised ecape function to use the new _entrainment class for better encapsulation and clarity.
- Improved nantrapz function by adding where parameter to exclude certain values from integration.
Tests:
- Added new tests for pressure_vector and height_vector to ensure correct functionality of comparison and conversion methods.
- Updated existing tests to accommodate changes in pressure_vector and height_vector implementations.

sourcery-ai · 2024-07-04T10:55:10Z

Reviewer's Guide by Sourcery

This pull request introduces vertical_vector, pressure_vector, and height_vector classes to handle pressure and height comparisons and conversions. It updates various functions across the codebase to use these new classes, ensuring more robust and readable code. Additionally, it includes new and updated tests to verify the functionality of these classes.

File-Level Changes

Files	Changes
`src/nzthermo/_core.pyx` `src/nzthermo/entrainment.py` `src/nzthermo/_core.pyi` `src/nzthermo/functional.py` `src/nzthermo/_ufunc.pyx` `src/nzthermo/_ufunc.pyi` `src/nzthermo/core.py` `src/nzthermo/__init__.py`	Introduced vertical_vector, pressure_vector, and height_vector classes and updated various functions to use these new classes for pressure and height comparisons and conversions.
`tests/core_test.py` `tests/ufunc_test.py` `tests/entrainment_test.py`	Added and updated tests to cover new functionalities of pressure_vector and height_vector classes.

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @leaver2000 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 5 issues found
🟢 Security: all looks good
🟡 Testing: 2 issues found
🟡 Complexity: 1 issue found
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-07-04T10:56:17Z

src/nzthermo/_core.pyx

 # ............................................................................................... #
 # helpers
 # ............................................................................................... #
+class vertical_vector(np.ndarray[_S, np.dtype[_T]], abc.ABC):


suggestion: Consider using a more descriptive class name.

The class name vertical_vector is somewhat generic. Consider using a more descriptive name that better conveys its purpose or functionality.

Suggested change

class vertical_vector(np.ndarray[_S, np.dtype[_T]], abc.ABC):

class VerticalVector(np.ndarray[_S, np.dtype[_T]], abc.ABC):

sourcery-ai · 2024-07-04T10:56:17Z

src/nzthermo/_core.pyx

+
+        return np.asarray(greater_or_close(self, top), np.bool_)
+
+    def is_between(self, bottom, top, close=False):


issue (bug_risk): Potential logical error in is_between method.

The condition self < bottom & self > top seems logically incorrect. It should likely be self > bottom & self < top.

sourcery-ai · 2024-07-04T10:56:17Z

src/nzthermo/entrainment.py

@@ -81,14 +79,14 @@ def _get_profile(
    return parcel_profile(pressure, t, td)


-class entrainment(Generic[_T]):
+class _entrainment(Generic[_T]):


question: Class name starting with an underscore.

The class name _entrainment starts with an underscore, which typically indicates a private class. If this is not intended to be private, consider renaming it.

sourcery-ai · 2024-07-04T10:56:17Z

src/nzthermo/functional.py

@@ -108,6 +108,13 @@ def nanroll_2d(
    return args


+def arange_slice(x: slice, nd: int, axis: SupportsIndex = -1) -> tuple[slice, ...]:
+    s = [slice(None)] * nd


suggestion: Consider renaming variable s.

The variable name s is not very descriptive. Consider renaming it to something more meaningful, like slices.

Suggested change

s = [slice(None)] * nd

slices = [slice(None)] * nd

sourcery-ai · 2024-07-04T10:56:17Z

src/nzthermo/_core.pyi

+_T = TypeVar("_T")
+_DType_T_co = TypeVar("_DType_T_co", bound=np.dtype[Any])
+
+class vertical_vector(np.ndarray[_S, _DType_T_co], abc.ABC):


suggestion: Consider adding a docstring for the class.

Adding a docstring for the vertical_vector class can help other developers understand its purpose and usage.

Suggested change

class vertical_vector(np.ndarray[_S, _DType_T_co], abc.ABC):

class vertical_vector(np.ndarray[_S, _DType_T_co], abc.ABC):

"""

A class representing a vertical vector, inheriting from numpy's ndarray.

This class is designed to handle vertical vector operations with specific

data types and shapes.

"""

sourcery-ai · 2024-07-04T10:56:18Z

tests/core_test.py

+def test_pressure_vector() -> None:
+    p = pressure_vector(
+        [
+            101300.0,
+            100000.0,
+            97500.0,
+            95000.0,
+            92500.0,
+            90000.0,
+            87500.0,


suggestion (testing): Consider adding edge case tests for pressure_vector

The test_pressure_vector function is comprehensive, but it would be beneficial to add edge cases such as empty arrays, arrays with a single element, and arrays with non-monotonic sequences to ensure robustness.

sourcery-ai · 2024-07-04T10:56:18Z

tests/entrainment_test.py

@@ -42,7 +42,7 @@
 V = data["V"][step]
 print(np.abs(Q - data["Q"][step]).max())

-MSL: np.ndarray = data["Z"][step]
+MSL: height_vector = data["Z"][step].view(height_vector)


suggestion (testing): Add test for height_vector edge cases

Consider adding tests to validate edge cases for height_vector, such as empty arrays, arrays with a single element, and arrays with non-monotonic sequences. This will help ensure the height_vector class handles these scenarios correctly.

sourcery-ai · 2024-07-04T10:56:18Z

src/nzthermo/functional.py

@@ -108,6 +108,13 @@
    return args


+def arange_slice(x: slice, nd: int, axis: SupportsIndex = -1) -> tuple[slice, ...]:


issue (complexity): Consider simplifying the code to maintain readability and reduce unnecessary complexity.

The new code introduces additional complexity that could be avoided. Here are some points to consider:

Introduction of arange_slice Function: While the new arange_slice function aims for reusability, it adds an extra layer of abstraction that makes the code harder to follow. The original inline slice creation was more straightforward and easier to understand.

Increased Indirection: Using the arange_slice function to create slices (s1 and s2) adds an extra step for the reader to understand what these slices are doing. The original code directly created the slices, making it immediately clear what they represent.

Changes in Variable Names: The variable names have been changed from slice1 and slice2 to s1 and s2. While shorter, these names are less descriptive and can make the code harder to understand at a glance.

Tuple Creation: The new code uses tuple creation in the zero_crossings function, which adds unnecessary complexity. The original code was more straightforward in its approach.

Consider simplifying the code to maintain readability and reduce unnecessary complexity.

sourcery-ai · 2024-07-04T10:56:18Z

tests/core_test.py

+    assert isinstance(p2, pressure_vector)
+    assert np.allclose(p, p2)
+
+    assert np.all(p.is_above(101325.0))


issue (code-quality): Extract duplicate code into function (extract-duplicate-method)

🔢pressure and height vectors

235fb06

leaver2000 merged commit 3435f97 into master Jul 4, 2024
1 check failed

sourcery-ai bot reviewed Jul 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔢pressure and height vectors #25

🔢pressure and height vectors #25

leaver2000 commented Jul 4, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jul 4, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

sourcery-ai bot Jul 4, 2024

	class vertical_vector(np.ndarray[_S, np.dtype[_T]], abc.ABC):
	class VerticalVector(np.ndarray[_S, np.dtype[_T]], abc.ABC):


		return np.asarray(greater_or_close(self, top), np.bool_)

		def is_between(self, bottom, top, close=False):

-class vertical_vector(np.ndarray[_S, _DType_T_co], abc.ABC):
+class vertical_vector(np.ndarray[_S, _DType_T_co], abc.ABC):
+    """
+    A class representing a vertical vector, inheriting from numpy's ndarray.
+    This class is designed to handle vertical vector operations with specific
+    data types and shapes.
+    """

		@@ -108,6 +108,13 @@
		return args


		def arange_slice(x: slice, nd: int, axis: SupportsIndex = -1) -> tuple[slice, ...]:

🔢pressure and height vectors #25

🔢pressure and height vectors #25

Conversation

leaver2000 commented Jul 4, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Jul 4, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 4, 2024

Choose a reason for hiding this comment

leaver2000 commented Jul 4, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jul 4, 2024 •

edited

Loading