-
Notifications
You must be signed in to change notification settings - Fork 92
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
361 test geometries with hypothesis #364
Changes from 13 commits
eca9807
cb6bd77
aa03580
c21b1fc
7acfb30
78774c5
4dbdb9e
6e8db13
9edd464
b73fcc5
8aab9e6
015cdd1
8b5f36a
b9eda46
fd79e87
1142a1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,6 +93,8 @@ | |
|
||
MsgMutualExclusive: Final = "Geometry and kml coordinates are mutually exclusive" | ||
|
||
xml_attrs = {"ns", "name_spaces", "id", "target_id"} | ||
|
||
|
||
def handle_invalid_geometry_error( | ||
*, | ||
|
@@ -159,15 +161,14 @@ | |
|
||
""" | ||
if getattr(obj, attr_name, None): | ||
p = precision if precision is not None else 6 | ||
coords = getattr(obj, attr_name) | ||
if len(coords[0]) == 2: # noqa: PLR2004 | ||
tuples = (f"{c[0]:.{p}f},{c[1]:.{p}f}" for c in coords) | ||
elif len(coords[0]) == 3: # noqa: PLR2004 | ||
tuples = (f"{c[0]:.{p}f},{c[1]:.{p}f},{c[2]:.{p}f}" for c in coords) | ||
else: | ||
if not coords or len(coords[0]) not in (2, 3): | ||
msg = f"Invalid dimensions in coordinates '{coords}'" | ||
raise KMLWriteError(msg) | ||
if precision is None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (performance): Move the dimension check to the beginning of the function Consider moving the check for valid dimensions (len(coords[0]) not in (2, 3)) to the beginning of the function. This would prevent unnecessary string formatting for invalid input, improving efficiency.
|
||
tuples = (",".join(str(c) for c in coord) for coord in coords) | ||
else: | ||
tuples = (",".join(f"{c:.{precision}f}" for c in coord) for coord in coords) | ||
element.text = " ".join(tuples) | ||
|
||
|
||
|
@@ -488,6 +489,21 @@ | |
""" | ||
return bool(self.geometry) | ||
|
||
def __eq__(self, other: object) -> bool: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Extract common equality check logic into a utility function The eq methods for Point, LineString, and Polygon are very similar. Consider extracting this common logic into a utility function to reduce code duplication and improve maintainability.
|
||
"""Check if the Point objects are equal.""" | ||
if isinstance(other, Point): | ||
return all( | ||
getattr(self, attr) == getattr(other, attr) | ||
for attr in ( | ||
"extrude", | ||
"altitude_mode", | ||
"geometry", | ||
*xml_attrs, | ||
*self._get_splat(), | ||
) | ||
) | ||
return super().__eq__(other) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Increase test coverage for Static analysis indicates that line 505 ( 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
|
||
Comment on lines
+492
to
+506
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note on Test Coverage for The Would you like assistance in creating a unit test to cover this scenario? 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
@property | ||
def geometry(self) -> Optional[geo.Point]: | ||
""" | ||
|
@@ -628,6 +644,21 @@ | |
""" | ||
return bool(self.geometry) | ||
|
||
def __eq__(self, other: object) -> bool: | ||
"""Check if the LineString objects is equal.""" | ||
if isinstance(other, LineString): | ||
return all( | ||
getattr(self, attr) == getattr(other, attr) | ||
for attr in ( | ||
"extrude", | ||
"tessellate", | ||
"geometry", | ||
*xml_attrs, | ||
*self._get_splat(), | ||
) | ||
) | ||
return super().__eq__(other) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Increase test coverage for Line 660 is not covered by tests. Please add a unit test where 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
|
||
Comment on lines
+647
to
+661
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note on Test Coverage for Similarly, in the Can I help in drafting a unit test for this case? 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
@property | ||
def geometry(self) -> Optional[geo.LineString]: | ||
""" | ||
|
@@ -754,22 +785,6 @@ | |
**kwargs, | ||
) | ||
|
||
def __repr__(self) -> str: | ||
"""Create a string (c)representation for LinearRing.""" | ||
return ( | ||
f"{self.__class__.__module__}.{self.__class__.__name__}(" | ||
f"ns={self.ns!r}, " | ||
f"name_spaces={self.name_spaces!r}, " | ||
f"id={self.id!r}, " | ||
f"target_id={self.target_id!r}, " | ||
f"extrude={self.extrude!r}, " | ||
f"tessellate={self.tessellate!r}, " | ||
f"altitude_mode={self.altitude_mode}, " | ||
f"geometry={self.geometry!r}, " | ||
f"**{self._get_splat()!r}," | ||
")" | ||
) | ||
|
||
@property | ||
def geometry(self) -> Optional[geo.LinearRing]: | ||
""" | ||
|
@@ -1182,12 +1197,26 @@ | |
f"extrude={self.extrude!r}, " | ||
f"tessellate={self.tessellate!r}, " | ||
f"altitude_mode={self.altitude_mode}, " | ||
f"outer_boundary={self.outer_boundary!r}, " | ||
f"inner_boundaries={self.inner_boundaries!r}, " | ||
f"geometry={self.geometry!r}, " | ||
f"**{self._get_splat()!r}," | ||
")" | ||
) | ||
|
||
def __eq__(self, other: object) -> bool: | ||
"""Check if the Polygon objects are equal.""" | ||
if isinstance(other, Polygon): | ||
return all( | ||
getattr(self, attr) == getattr(other, attr) | ||
for attr in ( | ||
"extrude", | ||
"tessellate", | ||
"geometry", | ||
*xml_attrs, | ||
*self._get_splat(), | ||
) | ||
) | ||
return super().__eq__(other) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Increase test coverage for Line 1218 lacks test coverage. Including a test case where 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
|
||
Comment on lines
+1205
to
+1219
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note on Test Coverage for The else branch in the Would you like me to assist in writing this unit test? 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
|
||
registry.register( | ||
Polygon, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
""" | ||
Configure the tests. | ||
|
||
Register the hypothesis 'exhaustive' profile to run 10 thousand examples. | ||
Run this profile with ``pytest --hypothesis-profile=exhaustive`` | ||
""" | ||
|
||
from hypothesis import HealthCheck | ||
from hypothesis import settings | ||
|
||
settings.register_profile( | ||
"exhaustive", | ||
max_examples=10_000, | ||
suppress_health_check=[HealthCheck.too_slow], | ||
) | ||
settings.register_profile( | ||
"coverage", | ||
max_examples=10, | ||
suppress_health_check=[HealthCheck.too_slow], | ||
) | ||
Comment on lines
+16
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider increasing max_examples for the "coverage" profile. While the current setting of 10 examples is good for quick checks, it might not provide comprehensive coverage for more complex scenarios. Consider increasing this value to strike a balance between speed and thorough coverage. You might want to try a value between 50 and 100, for example: settings.register_profile(
"coverage",
max_examples=50,
suppress_health_check=[HealthCheck.too_slow],
) |
||
settings.register_profile("ci", suppress_health_check=[HealthCheck.too_slow]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider explicitly setting max_examples for the "ci" profile. The "ci" profile currently doesn't specify a max_examples value, which means it will use Hypothesis's default. For consistency with other profiles and to ensure predictable behavior in CI environments, consider explicitly setting this value. You might want to add a max_examples parameter: settings.register_profile(
"ci",
max_examples=200, # Adjust this value based on your CI requirements
suppress_health_check=[HealthCheck.too_slow],
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Inconsistent Python Version Specifications
The
pyupgrade
hook in.pre-commit-config.yaml
has been updated to include--py38-plus
, indicating a shift to Python 3.8+. However, no minimum Python version specification was found in other configuration files (e.g.,pyproject.toml
, CI workflows). Please ensure that all relevant configuration files specify Python 3.8+ to maintain consistency across the project.🔗 Analysis chain
Upgrade of minimum supported Python version
The
pyupgrade
hook arguments have been updated from--py37-plus
to--py38-plus
, indicating an increase in the minimum supported Python version to 3.8. This change aligns with modern Python practices and likely takes advantage of features introduced in Python 3.8.To ensure consistency across the project, let's verify the Python version specification in other configuration files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 2380
Script:
Length of output: 1727