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

216 add output verbosity #360

Merged
merged 18 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions fastkml/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ def etree_element(
node_name=item.node_name,
precision=precision,
verbosity=verbosity,
default=item.default,
)
return element

Expand Down
2 changes: 1 addition & 1 deletion fastkml/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def _missing_(cls, value: object) -> "RelaxedEnum":
class Verbosity(Enum):
"""Enum to represent the different verbosity levels."""

quiet = 0
terse = 0
normal = 1
verbose = 2

Expand Down
7 changes: 6 additions & 1 deletion fastkml/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ def __repr__(self) -> str:
classes=(bool,),
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent Default Value for visibility in _Feature

The visibility attribute is assigned a default value of True in the registry.register call at line 356. However, in the _Feature class __init__ method, visibility defaults to None. For consistency and to prevent potential confusion, consider setting the default value of visibility to True in the __init__ method as well.

Apply this diff to align the default values:

 def __init__(
     ...
-    visibility: Optional[bool] = None,
+    visibility: Optional[bool] = True,
     ...
 ) -> None:
     ...
     self.visibility = visibility

Committable suggestion was skipped due to low confidence.

),
)
registry.register(
Expand All @@ -364,6 +365,7 @@ def __repr__(self) -> str:
classes=(bool,),
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent Default Value for isopen in _Feature

Similarly, the isopen attribute has a default value of False in the registry.register call at line 368, but defaults to None in the __init__ method. To maintain consistency and ensure predictable behavior, consider setting the default value of isopen to False in the __init__ method.

Apply this diff to synchronize the default values:

 def __init__(
     ...
-    isopen: Optional[bool] = None,
+    isopen: Optional[bool] = False,
     ...
 ) -> None:
     ...
     self.isopen = isopen

Committable suggestion was skipped due to low confidence.

),
)
registry.register(
Expand Down Expand Up @@ -745,7 +747,8 @@ class NetworkLink(_Feature):
For example, Google Earth would fly to the <LookAt> view of the parent Document,
not the <LookAt> of the Placemarks contained within the Document.
<Link>(required)
https://developers.google.com/kml/documentation/kmlreference#link

https://developers.google.com/kml/documentation/kmlreference#networklink
"""
Comment on lines +751 to 752
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Typographical Error in NetworkLink Docstring

At lines 750-751, the docstring for the NetworkLink class appears to have an unintended indentation or misplaced closing triple quotes. The URL should be included within the docstring, and the closing triple quotes should align with the opening ones.

Apply this diff to correct the docstring:

     <Link>(required)
-        """
+        https://developers.google.com/kml/documentation/kmlreference#networklink
+    """

Ensure that the docstring is properly enclosed and indented.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
https://developers.google.com/kml/documentation/kmlreference#networklink
"""
<Link>(required)
https://developers.google.com/kml/documentation/kmlreference#networklink
"""


refresh_visibility: Optional[bool]
Expand Down Expand Up @@ -921,6 +924,7 @@ def __bool__(self) -> bool:
classes=(bool,),
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent Default Value for refresh_visibility in NetworkLink

The refresh_visibility attribute is assigned a default value of False in the registry.register call at line 927. However, in the NetworkLink class __init__ method, refresh_visibility defaults to None. For consistency and to prevent potential confusion, consider setting the default value of refresh_visibility to False in the __init__ method as well.

Apply this diff to align the default values:

 def __init__(
     ...
-    refresh_visibility: Optional[bool] = None,
+    refresh_visibility: Optional[bool] = False,
     ...
 ) -> None:
     ...
     self.refresh_visibility = refresh_visibility

Committable suggestion was skipped due to low confidence.

),
)
registry.register(
Expand All @@ -932,6 +936,7 @@ def __bool__(self) -> bool:
classes=(bool,),
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent Default Value for fly_to_view in NetworkLink

Similarly, the fly_to_view attribute has a default value of False in the registry.register call at line 939, but it defaults to None in the __init__ method. To maintain consistency and ensure predictable behavior, consider setting the default value of fly_to_view to False in the __init__ method.

Apply this diff to synchronize the default values:

 def __init__(
     ...
-    fly_to_view: Optional[bool] = None,
+    fly_to_view: Optional[bool] = False,
     ...
 ) -> None:
     ...
     self.fly_to_view = fly_to_view

Committable suggestion was skipped due to low confidence.

),
)
registry.register(
Expand Down
144 changes: 93 additions & 51 deletions fastkml/geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def coordinates_subelement(
node_name: str, # noqa: ARG001
precision: Optional[int],
verbosity: Optional[Verbosity], # noqa: ARG001
default: Any, # noqa: ARG001
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Unused default parameter in coordinates_subelement function

The default parameter has been added to the coordinates_subelement function but is not used within the function body. This could lead to confusion and should be removed to clean up the function signature.

Apply this diff to remove the unused parameter:

 def coordinates_subelement(
     obj: _XMLObject,
     *,
     element: Element,
     attr_name: str,
     node_name: str,  # noqa: ARG001
     precision: Optional[int],
     verbosity: Optional[Verbosity],  # noqa: ARG001
-    default: Any,  # noqa: ARG001
 ) -> None:
     """
     Set the value of an attribute from a subelement with a text node.

     Args:
     ----
         obj (_XMLObject): The object from which to retrieve the attribute value.
         element (Element): The parent element to add the subelement to.
         attr_name (str): The name of the attribute to retrieve the value from.
         node_name (str): The name of the subelement to create.
         precision (Optional[int]): The precision of the attribute value.
         verbosity (Optional[Verbosity]): The verbosity level.
-        default (Any): The default value of the attribute (unused).

Alternatively, if the default parameter is intended for future use or for interface consistency, consider prefixing it with an underscore to indicate that it is intentionally unused:

-    default: Any,  # noqa: ARG001
+    _default: Any,  # noqa: ARG001

Also applies to: 154-154

) -> None:
"""
Set the value of an attribute from a subelement with a text node.
Expand All @@ -150,6 +151,7 @@ def coordinates_subelement(
node_name (str): The name of the subelement to create.
precision (Optional[int]): The precision of the attribute value.
verbosity (Optional[Verbosity]): The verbosity level.
default (Any): The default value of the attribute (unused).

Returns:
-------
Expand Down Expand Up @@ -318,8 +320,6 @@ class _Geometry(_BaseObject):

"""

extrude: Optional[bool]
tessellate: Optional[bool]
altitude_mode: Optional[AltitudeMode]

def __init__(
Expand All @@ -329,8 +329,6 @@ def __init__(
name_spaces: Optional[Dict[str, str]] = None,
id: Optional[str] = None,
target_id: Optional[str] = None,
extrude: Optional[bool] = None,
tessellate: Optional[bool] = None,
altitude_mode: Optional[AltitudeMode] = None,
**kwargs: Any,
) -> None:
Expand All @@ -357,8 +355,6 @@ def __init__(
target_id=target_id,
**kwargs,
)
self.extrude = extrude
self.tessellate = tessellate
self.altitude_mode = altitude_mode

def __repr__(self) -> str:
Expand All @@ -369,36 +365,12 @@ def __repr__(self) -> str:
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"**{self._get_splat()!r},"
")"
)


registry.register(
_Geometry,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="extrude",
node_name="extrude",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
),
)
registry.register(
_Geometry,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="tessellate",
node_name="tessellate",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
),
)
registry.register(
_Geometry,
item=RegistryItem(
Expand All @@ -408,6 +380,7 @@ def __repr__(self) -> str:
node_name="altitudeMode",
get_kwarg=subelement_enum_kwarg,
set_element=enum_subelement,
default=AltitudeMode.clamp_to_ground,
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Potential Conflicts with Existing altitude_mode Settings

The verification process identified several instances where altitude_mode is explicitly set to values other than AltitudeMode.clamp_to_ground. Specifically:

  • AltitudeMode.relative_to_ground
  • AltitudeMode.clamp_to_seaFloor
  • AltitudeMode.absolute

These variations indicate that setting the default altitude_mode to clamp_to_ground may conflict with existing geometries that rely on different altitude modes. It's recommended to review these instances to ensure that the default setting aligns with their intended behaviors.

Affected Files:

  • tests/geometries/geometry_test.py
  • tests/gx_test.py
  • tests/overlays_test.py
  • tests/repr_eq_test.py
  • tests/views_test.py
  • fastkml/views.py
  • fastkml/gx.py
  • fastkml/geometry.py
🔗 Analysis chain

Setting default altitude_mode for _Geometry

Adding default=AltitudeMode.clamp_to_ground in the registry registration sets the default altitude_mode for all geometries derived from _Geometry. Ensure that this default aligns with the intended behavior across all geometry types.

Run the following script to verify that setting the default altitude_mode does not conflict with existing geometries:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find instances where 'altitude_mode' is explicitly set in geometries.

# Test: Search for explicit assignments of 'altitude_mode'.
# Expect: Verify whether any geometries rely on a different default value.
rg --type py 'altitude_mode\s*=' -A 2 -B 2

Length of output: 62


Script:

#!/bin/bash
# Description: 
# 1. Find instances where 'altitude_mode' is explicitly set in geometries.
# 2. Verify if there are any subclasses of '_Geometry' that might override the default.

# Search for explicit assignments of 'altitude_mode'.
rg --type py 'altitude_mode\s*=' -A 2 -B 2

# Search for subclasses of '_Geometry'.
rg --type py 'class\s+\w+\(.*_Geometry.*\):' -A 5 -B 0

Length of output: 28845

),
)

Expand All @@ -424,6 +397,7 @@ class Point(_Geometry):
https://developers.google.com/kml/documentation/kmlreference#point
"""

extrude: Optional[bool]
kml_coordinates: Optional[Coordinates]

def __init__(
Expand All @@ -434,7 +408,6 @@ def __init__(
id: Optional[str] = None,
target_id: Optional[str] = None,
extrude: Optional[bool] = None,
tessellate: Optional[bool] = None,
altitude_mode: Optional[AltitudeMode] = None,
geometry: Optional[geo.Point] = None,
kml_coordinates: Optional[Coordinates] = None,
Expand Down Expand Up @@ -471,13 +444,13 @@ def __init__(
else None
)
self.kml_coordinates = kml_coordinates
self.extrude = extrude
kwargs.pop("tessellate", None)
super().__init__(
ns=ns,
id=id,
name_spaces=name_spaces,
target_id=target_id,
extrude=extrude,
tessellate=tessellate,
altitude_mode=altitude_mode,
**kwargs,
)
Expand All @@ -498,7 +471,6 @@ def __repr__(self) -> str:
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"kml_coordinates={self.kml_coordinates!r}, "
f"**{self._get_splat()!r},"
Expand Down Expand Up @@ -546,6 +518,18 @@ def geometry(self) -> Optional[geo.Point]:
set_element=xml_subelement,
),
)
registry.register(
Point,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="extrude",
node_name="extrude",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
),
)


class LineString(_Geometry):
Expand All @@ -562,6 +546,10 @@ class LineString(_Geometry):
https://developers.google.com/kml/documentation/kmlreference#linestring
"""

extrude: Optional[bool]
tessellate: Optional[bool]
kml_coordinates: Optional[Coordinates]

def __init__(
self,
*,
Expand Down Expand Up @@ -602,13 +590,13 @@ def __init__(
if kml_coordinates is None:
kml_coordinates = Coordinates(coords=geometry.coords) if geometry else None
self.kml_coordinates = kml_coordinates
self.extrude = extrude
self.tessellate = tessellate
super().__init__(
ns=ns,
name_spaces=name_spaces,
id=id,
target_id=target_id,
extrude=extrude,
tessellate=tessellate,
altitude_mode=altitude_mode,
**kwargs,
)
Expand Down Expand Up @@ -671,6 +659,31 @@ def geometry(self) -> Optional[geo.LineString]:
),
)

registry.register(
LineString,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="extrude",
node_name="extrude",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
),
)
registry.register(
LineString,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="tessellate",
node_name="tessellate",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
),
)


class LinearRing(LineString):
"""
Expand Down Expand Up @@ -1034,6 +1047,8 @@ class Polygon(_Geometry):
https://developers.google.com/kml/documentation/kmlreference#polygon
"""

extrude: Optional[bool]
tessellate: Optional[bool]
outer_boundary: Optional[OuterBoundaryIs]
inner_boundaries: List[InnerBoundaryIs]

Expand Down Expand Up @@ -1099,13 +1114,13 @@ def __init__(
]
self.outer_boundary = outer_boundary
self.inner_boundaries = list(inner_boundaries) if inner_boundaries else []
self.extrude = extrude
self.tessellate = tessellate
super().__init__(
ns=ns,
name_spaces=name_spaces,
id=id,
target_id=target_id,
extrude=extrude,
tessellate=tessellate,
altitude_mode=altitude_mode,
**kwargs,
)
Expand Down Expand Up @@ -1196,6 +1211,30 @@ def __repr__(self) -> str:
set_element=xml_subelement_list,
),
)
registry.register(
Polygon,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="extrude",
node_name="extrude",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
),
)
registry.register(
Polygon,
item=RegistryItem(
ns_ids=("kml",),
classes=(bool,),
attr_name="tessellate",
node_name="tessellate",
get_kwarg=subelement_bool_kwarg,
set_element=bool_subelement,
default=False,
),
)


def create_multigeometry(
Expand Down Expand Up @@ -1294,7 +1333,7 @@ def create_kml_geometry(
raise KMLWriteError(msg) # pragma: no cover


class MultiGeometry(_Geometry):
class MultiGeometry(_BaseObject):
"""A container for zero or more geometry primitives."""

kml_geometries: List[Union[Point, LineString, Polygon, LinearRing, Self]]
Expand Down Expand Up @@ -1328,17 +1367,26 @@ def __init__(
The ID of the KML element.
target_id : str, optional
The target ID of the KML element.
extrude : bool, optional
Specifies whether to extend the geometry to the ground.
tessellate : bool, optional
Specifies whether to allow the geometry to follow the terrain.
altitude_mode : AltitudeMode, optional
The altitude mode of the geometry.
kml_geometries : iterable of Point, LineString, Polygon, LinearRing,
MultiGeometry
A collection of KML geometries.
geometry : MultiGeometryType, optional
A multi-geometry object.
Parameters for geometry and kml_geometries are mutually exclusive.
When geometry is provided, kml_geometries will be created from it and
you can specify additional parameters like extrude, tessellate, and
altitude_mode which will be set on the individual geometries.
extrude : bool, optional
Specifies whether to extend the geometry to the ground.
This is not set on the multi-geometry itself, but on the individual
geometries.
tessellate : bool, optional
Specifies whether to allow the geometry to follow the terrain.
This is not set on the multi-geometry itself, but on the individual
geometries.
altitude_mode : AltitudeMode, optional
The altitude mode of the geometry. This is not set on the multi-geometry
itself, but on the individual geometries.
**kwargs : any
Additional keyword arguments.

Expand Down Expand Up @@ -1373,9 +1421,6 @@ def __init__(
name_spaces=name_spaces,
id=id,
target_id=target_id,
extrude=extrude,
tessellate=tessellate,
altitude_mode=altitude_mode,
**kwargs,
)

Expand All @@ -1391,9 +1436,6 @@ def __repr__(self) -> str:
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"kml_geometries={self.kml_geometries!r}, "
f"**{self._get_splat()!r},"
")"
Expand Down
Loading
Loading