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.

Sign up for GitHub

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

374 add hypothesis tests for times and views #379

Merged
merged 5 commits into from
Nov 7, 2024
Merged

374 add hypothesis tests for times and views #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
45bbc7b
add hypothesis tests for TimeStamp and TimeSpan
cleder Nov 5, 2024
44978db
enhance hypothesis tests: Allow None values for timestamp, begin, and…
cleder Nov 5, 2024
53aef73
add hypothesis tests for views
cleder Nov 7, 2024
871fe7c
Merge branch 'develop' into 374-add-hypothesis-tests-links
cleder Nov 7, 2024
36f0573
fix doctest
cleder Nov 7, 2024
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
2 changes: 1 addition & 1 deletion docs/working_with_kml.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ CascadingStyle element into a supported Style element.
<kml:altitude>13.96486261382906</kml:altitude>
<kml:heading>0.0</kml:heading>
<kml:tilt>0.0</kml:tilt>
<kml:altitudeMode>absolute</kml:altitudeMode>
<kml:range>632.584179697442</kml:range>
<kml:altitudeMode>absolute</kml:altitudeMode>
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

Documentation change to absolute altitudeMode is inconsistent with implementation

The documentation change to use absolute altitudeMode in the example doesn't align with the codebase implementation, where:

  1. The default altitudeMode across the codebase is clampToGround (see fastkml/views.py, fastkml/geometry.py, etc.)
  2. Test cases primarily use relativeToGround or clampToGround (see tests/views_test.py, tests/overlays_test.py)
  3. The only usage of absolute mode is in specific test cases, not as a default or common pattern

The documentation should maintain consistency with the codebase's default behavior and common usage patterns.

🔗 Analysis chain

Verify altitudeMode documentation consistency with code changes.

The change from an unspecified previous value to absolute in the documentation example needs verification to ensure it accurately reflects the current implementation in the views module.

Let's verify the altitudeMode implementation in the views module:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation of altitudeMode in view-related classes

# Test 1: Search for altitudeMode in view classes
rg -A 5 "altitudeMode" "src/**/views.py"

# Test 2: Search for altitude_mode attribute declarations
ast-grep --pattern 'class $_ {
  $$$
  altitude_mode: $_
  $$$
}'

Length of output: 164

Script:

#!/bin/bash
# Let's try a broader search approach

# Search for any Python files containing altitudeMode
rg -t py "altitudeMode" -A 5

# Search for KML-related files that might contain the implementation
fd -e py -e pyx -e pxd | xargs rg "(?i)kml.*altitude"

# Look for any test files that might verify altitude mode behavior
fd -e py "test" | xargs rg "altitudeMode"

# Search for any configuration or constant definitions related to altitude modes
rg -t py "(?i)altitude.*mode.*=|ALTITUDE.*MODE"

Length of output: 45072

</kml:LookAt>
<kml:styleUrl>#__managed_style_0D301BCC0014827EFCCB</kml:styleUrl>
<kml:Polygon>
Expand Down
23 changes: 19 additions & 4 deletions fastkml/times.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,30 @@ def __init__(
resolution: Optional[DateTimeResolution] = None,
) -> None:
"""Initialize a KmlDateTime object."""
self.dt = dt
self.resolution = resolution
if resolution is None:
# sourcery skip: swap-if-expression
self.resolution = (
resolution = (
DateTimeResolution.date
if not isinstance(dt, datetime)
else DateTimeResolution.datetime
)
dt = (
dt.date()
if isinstance(dt, datetime) and resolution != DateTimeResolution.datetime
else dt
)
if resolution == DateTimeResolution.year:
self.dt = date(dt.year, 1, 1)
elif resolution == DateTimeResolution.year_month:
self.dt = date(dt.year, dt.month, 1)
else:
self.dt = dt
self.resolution = (
DateTimeResolution.date
if not isinstance(self.dt, datetime)
and resolution == DateTimeResolution.datetime
else resolution
)

def __repr__(self) -> str:
"""Create a string (c)representation for KmlDateTime."""
Expand Down Expand Up @@ -142,7 +157,7 @@ def parse(cls, datestr: str) -> Optional["KmlDateTime"]:
year = int(year_month_day_match.group("year"))
cleder marked this conversation as resolved.
Show resolved Hide resolved
month = int(year_month_day_match.group("month") or 1)
day = int(year_month_day_match.group("day") or 1)
dt = arrow.get(year, month, day).datetime
dt = date(year, month, day)
resolution = DateTimeResolution.date
if year_month_day_match.group("day") is None:
resolution = DateTimeResolution.year_month
Expand Down
91 changes: 35 additions & 56 deletions fastkml/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from typing import Any
from typing import Dict
from typing import Optional
from typing import Union

from fastkml import config
from fastkml.base import _XMLObject
Expand All @@ -35,8 +34,8 @@
from fastkml.mixins import TimeMixin
from fastkml.registry import RegistryItem
from fastkml.registry import registry
from fastkml.times import TimeSpan
from fastkml.times import TimeStamp

__all__ = ["Camera", "LatLonAltBox", "Lod", "LookAt", "Region"]

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -100,7 +99,6 @@ def __init__(
heading: Optional[float] = None,
tilt: Optional[float] = None,
altitude_mode: Optional[AltitudeMode] = None,
time_primitive: Union[TimeSpan, TimeStamp, None] = None,
**kwargs: Any,
) -> None:
"""
Expand Down Expand Up @@ -128,8 +126,6 @@ def __init__(
The tilt angle of the view.
altitude_mode : Optional[AltitudeMode]
The altitude mode of the view.
time_primitive : Union[TimeSpan, TimeStamp, None]
The time primitive associated with the view.
kwargs : Any
Additional keyword arguments.

Expand All @@ -151,7 +147,6 @@ def __init__(
self.heading = heading
self.tilt = tilt
self.altitude_mode = altitude_mode
self.times = time_primitive

def __repr__(self) -> str:
"""Create a string (c)representation for _AbstractView."""
Expand All @@ -167,7 +162,6 @@ def __repr__(self) -> str:
f"heading={self.heading!r}, "
f"tilt={self.tilt!r}, "
f"altitude_mode={self.altitude_mode}, "
cleder marked this conversation as resolved.
Show resolved Hide resolved
f"time_primitive={self.times!r}, "
f"**{self._get_splat()!r},"
")"
)
Expand Down Expand Up @@ -233,29 +227,6 @@ def __repr__(self) -> str:
default=0.0,
),
)
registry.register(
_AbstractView,
RegistryItem(
ns_ids=("kml",),
attr_name="altitude_mode",
node_name="altitudeMode",
classes=(AltitudeMode,),
get_kwarg=subelement_enum_kwarg,
set_element=enum_subelement,
default=AltitudeMode.clamp_to_ground,
),
)
registry.register(
_AbstractView,
RegistryItem(
ns_ids=("kml",),
attr_name="time_primitive",
node_name="TimeStamp",
classes=(TimeSpan, TimeStamp),
get_kwarg=xml_subelement_kwarg,
set_element=xml_subelement,
),
)


class Camera(_AbstractView):
Expand Down Expand Up @@ -300,7 +271,6 @@ def __init__(
tilt: Optional[float] = None,
roll: Optional[float] = None,
altitude_mode: Optional[AltitudeMode] = None,
time_primitive: Union[TimeSpan, TimeStamp, None] = None,
**kwargs: Any,
) -> None:
"""
Expand All @@ -319,8 +289,6 @@ def __init__(
tilt (Optional[float]): The tilt of the view.
roll (Optional[float]): The roll of the view.
altitude_mode (AltitudeMode): The altitude mode of the view.
time_primitive (Union[TimeSpan, TimeStamp, None]): The time primitive of the
view.
**kwargs (Any): Additional keyword arguments.

Returns:
Expand All @@ -339,7 +307,6 @@ def __init__(
heading=heading,
tilt=tilt,
altitude_mode=altitude_mode,
time_primitive=time_primitive,
**kwargs,
)
self.roll = roll
Expand All @@ -359,7 +326,6 @@ def __repr__(self) -> str:
f"tilt={self.tilt!r}, "
f"roll={self.roll!r}, "
f"altitude_mode={self.altitude_mode}, "
f"time_primitive={self.times!r}, "
f"**{self._get_splat()!r},"
")"
)
Expand All @@ -377,6 +343,18 @@ def __repr__(self) -> str:
default=0.0,
),
)
registry.register(
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider extracting the altitude_mode registration logic into a reusable helper function

The altitude_mode registration is duplicated across Camera, LookAt, and LatLonAltBox classes. Consider creating a helper function to reduce duplication while preserving namespace flexibility:

def register_altitude_mode(cls, ns_ids=("kml",)):
    """Helper to register altitude_mode with custom namespaces."""
    registry.register(
        cls,
        RegistryItem(
            ns_ids=ns_ids,
            attr_name="altitude_mode",
            node_name="altitudeMode", 
            classes=(AltitudeMode,),
            get_kwarg=subelement_enum_kwarg,
            set_element=enum_subelement,
            default=AltitudeMode.clamp_to_ground,
        ),
    )

# Usage:
register_altitude_mode(Camera, ns_ids=("kml", "gx", ""))
register_altitude_mode(LookAt, ns_ids=("kml", "gx", ""))
register_altitude_mode(LatLonAltBox, ns_ids=("kml", "gx", ""))

This maintains the namespace customization while eliminating code duplication.

Camera,
RegistryItem(
ns_ids=("kml", "gx", ""),
attr_name="altitude_mode",
node_name="altitudeMode",
classes=(AltitudeMode,),
get_kwarg=subelement_enum_kwarg,
set_element=enum_subelement,
default=AltitudeMode.clamp_to_ground,
),
)


class LookAt(_AbstractView):
Expand Down Expand Up @@ -407,7 +385,6 @@ def __init__(
tilt: Optional[float] = None,
range: Optional[float] = None,
altitude_mode: Optional[AltitudeMode] = None,
time_primitive: Union[TimeSpan, TimeStamp, None] = None,
**kwargs: Any,
) -> None:
"""
Expand All @@ -427,7 +404,6 @@ def __init__(
range (Optional[float]): The range value.
altitude_mode (AltitudeMode): The altitude mode. Defaults to
AltitudeMode.relative_to_ground.
time_primitive (Union[TimeSpan, TimeStamp, None]): The time primitive.
**kwargs (Any): Additional keyword arguments.

Returns:
Expand All @@ -446,7 +422,6 @@ def __init__(
heading=heading,
tilt=tilt,
altitude_mode=altitude_mode,
time_primitive=time_primitive,
**kwargs,
)
self.range = range
Expand All @@ -466,7 +441,6 @@ def __repr__(self) -> str:
f"tilt={self.tilt!r}, "
f"range={self.range!r}, "
f"altitude_mode={self.altitude_mode}, "
f"time_primitive={self.times!r}, "
f"**{self._get_splat()!r},"
")"
)
Expand All @@ -483,6 +457,18 @@ def __repr__(self) -> str:
set_element=float_subelement,
),
)
registry.register(
LookAt,
RegistryItem(
ns_ids=("kml", "gx", ""),
attr_name="altitude_mode",
node_name="altitudeMode",
classes=(AltitudeMode,),
get_kwarg=subelement_enum_kwarg,
set_element=enum_subelement,
default=AltitudeMode.clamp_to_ground,
),
)


class LatLonAltBox(_XMLObject):
Expand Down Expand Up @@ -653,7 +639,7 @@ def __bool__(self) -> bool:
registry.register(
LatLonAltBox,
RegistryItem(
ns_ids=("kml",),
ns_ids=("kml", "gx", ""),
attr_name="altitude_mode",
node_name="altitudeMode",
classes=(AltitudeMode,),
Expand Down Expand Up @@ -701,10 +687,10 @@ def __init__(
ns (Optional[str]): The namespace for the view.
name_spaces (Optional[Dict[str, str]]): The dictionary of namespace prefixes
and URIs.
min_lod_pixels (Optional[float]): The minimum level of detail in pixels.
max_lod_pixels (Optional[float]): The maximum level of detail in pixels.
min_fade_extent (Optional[float]): The minimum fade extent in pixels.
max_fade_extent (Optional[float]): The maximum fade extent in pixels.
min_lod_pixels (Optional[int]): The minimum level of detail in pixels.
max_lod_pixels (Optional[int]): The maximum level of detail in pixels.
min_fade_extent (Optional[int]): The minimum fade extent in pixels.
max_fade_extent (Optional[int]): The maximum fade extent in pixels.
**kwargs (Any): Additional keyword arguments.

Returns:
Expand Down Expand Up @@ -775,8 +761,8 @@ def __bool__(self) -> bool:
attr_name="min_fade_extent",
node_name="minFadeExtent",
classes=(float,),
get_kwarg=subelement_float_kwarg,
set_element=float_subelement,
get_kwarg=subelement_int_kwarg,
set_element=int_subelement,
default=0,
),
)
Expand All @@ -787,8 +773,8 @@ def __bool__(self) -> bool:
attr_name="max_fade_extent",
node_name="maxFadeExtent",
classes=(float,),
get_kwarg=subelement_float_kwarg,
set_element=float_subelement,
get_kwarg=subelement_int_kwarg,
set_element=int_subelement,
default=0,
),
)
Expand Down Expand Up @@ -902,10 +888,3 @@ def __bool__(self) -> bool:
set_element=xml_subelement,
),
)


__all__ = [
"Camera",
"LookAt",
"Region",
]
6 changes: 1 addition & 5 deletions tests/hypothesis/geometry_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,7 @@
altitude_mode=st.one_of(
st.none(),
st.sampled_from(
(
AltitudeMode.absolute,
AltitudeMode.clamp_to_ground,
AltitudeMode.relative_to_ground,
),
AltitudeMode,
),
),
)
Expand Down
14 changes: 2 additions & 12 deletions tests/hypothesis/gx_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@
# along with this library; if not, write to the Free Software Foundation, Inc.,
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
"""Test gx Track and MultiTrack."""
import datetime
import typing

from hypothesis import given
from hypothesis import strategies as st
from hypothesis.extra.dateutil import timezones
from pygeoif.hypothesis.strategies import epsg4326
from pygeoif.hypothesis.strategies import points

Expand All @@ -29,12 +27,12 @@
import fastkml.types
from fastkml.gx import Angle
from fastkml.gx import TrackItem
from fastkml.times import KmlDateTime
from tests.base import Lxml
from tests.hypothesis.common import assert_repr_roundtrip
from tests.hypothesis.common import assert_str_roundtrip
from tests.hypothesis.common import assert_str_roundtrip_terse
from tests.hypothesis.common import assert_str_roundtrip_verbose
from tests.hypothesis.strategies import kml_datetimes
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Don't import test modules. (dont-import-test-modules)

ExplanationDon't import test modules.

Tests should be self-contained and don't depend on each other.

If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.

from tests.hypothesis.strategies import nc_name

track_items = st.builds(
Expand All @@ -51,15 +49,7 @@
),
),
coord=points(srs=epsg4326),
when=st.builds(
KmlDateTime,
dt=st.datetimes(
allow_imaginary=False,
timezones=timezones(),
min_value=datetime.datetime(2000, 1, 1), # noqa: DTZ001
max_value=datetime.datetime(2050, 1, 1), # noqa: DTZ001
),
),
when=kml_datetimes(),
)


Expand Down
8 changes: 1 addition & 7 deletions tests/hypothesis/multi_geometry_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,7 @@
tessellate=st.one_of(st.none(), st.booleans()),
altitude_mode=st.one_of(
st.none(),
st.sampled_from(
(
AltitudeMode.absolute,
AltitudeMode.clamp_to_ground,
AltitudeMode.relative_to_ground,
),
),
st.sampled_from(AltitudeMode),
),
)

Expand Down
Loading
Loading