-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add HAE and DEM projections to SICD sensor model #12
Changes from all commits
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 |
---|---|---|
@@ -1 +0,0 @@ | ||
# nothing here | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,22 @@ | ||
from abc import ABC, abstractmethod | ||
from dataclasses import dataclass | ||
from typing import Optional | ||
|
||
from .coordinates import GeodeticWorldCoordinate | ||
|
||
|
||
@dataclass | ||
class ElevationRegionSummary: | ||
""" | ||
This class contains a general summary of an elevation tile. | ||
""" | ||
|
||
min_elevation: float | ||
max_elevation: float | ||
no_data_value: int | ||
post_spacing: float | ||
|
||
|
||
class ElevationModel(ABC): | ||
""" | ||
An elevation model associates a height z for a given x, y of a world coordinate. It typically provides information | ||
|
@@ -24,6 +38,15 @@ def set_elevation(self, world_coordinate: GeodeticWorldCoordinate) -> None: | |
:return: None | ||
""" | ||
|
||
@abstractmethod | ||
def describe_region(self, world_coordinate: GeodeticWorldCoordinate) -> Optional[ElevationRegionSummary]: | ||
""" | ||
Get a summary of the region near the provided world coordinate | ||
|
||
:param world_coordinate: the coordinate at the center of the region of interest | ||
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. Just a small comment but I am not sure we should have a convention about these comments where we don't use "the/a/ect." to start sentences. I have also been using capitalized convention elsewhere - but it doesn't matter at much. 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. I don't think there is a standard convention for this. I likely just picked up the convention I use because it was in the examples I saw on the Sphinx website: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#field-lists The "the/a" convention is used by some projects (e.g. Flask: https://flask.palletsprojects.com/en/2.3.x/api/#flask.Flask.run) and not in others (e.g. NumPy: https://numpy.org/doc/stable/reference/generated/numpy.ndarray.html#numpy.ndarray). If you feel super strongly about this feel free to make a case for something the team should conform to. Until then I'd rather see the team focus on the quality / content of the documentation rather than a specific sentence structure. |
||
:return: the summary information | ||
""" | ||
|
||
|
||
class ConstantElevationModel(ElevationModel): | ||
""" | ||
|
@@ -50,3 +73,17 @@ def set_elevation(self, world_coordinate: GeodeticWorldCoordinate) -> None: | |
:return: None | ||
""" | ||
world_coordinate.elevation = self.constant_elevation | ||
|
||
def describe_region(self, world_coordinate: GeodeticWorldCoordinate) -> Optional[ElevationRegionSummary]: | ||
""" | ||
Get a summary of the region near the provided world coordinate | ||
|
||
:param world_coordinate: the coordinate at the center of the region of interest | ||
:return: [min elevation value, max elevation value, no elevation data value, post spacing] | ||
""" | ||
return ElevationRegionSummary( | ||
min_elevation=self.constant_elevation, | ||
max_elevation=self.constant_elevation, | ||
no_data_value=-32767, | ||
post_spacing=30.0, | ||
) |
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.
Could this Optional[Any] but a Optional[List[Float/Int]] perhaps?
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.
It should either be a NumPy ndarray or an ArrayLike. This is a method from the abstract base class defined in digital_elevation_model.py. That file has a note to clean up some of the typing now that we've upgraded to NumPy > 1.20 (where typing was added).
This change added the ElevationRegionSummary to this call but didn't change that portion of the signature. I created a ticket on our internal backlog to make sure this tech debt item doesn't fall through the cracks but it isn't part of this feature.