Skip to content

Commit

Permalink
check ncoefs only on attribute access
Browse files Browse the repository at this point in the history
  • Loading branch information
@raphaelquast
raphaelquast committed Nov 27, 2023
1 parent 9c94d84 commit f312819
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 14 deletions.
11 changes: 11 additions & 0 deletions src/rt1_model/_scatter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@
class _Scatter:
"""The base object for any Surface and Volume objects."""

@property
def ncoefs(self):
"""The number of coefficients used in the legendre expansion."""
if not hasattr(self, "_ncoefs") or self._ncoefs is None:

This comment was marked as resolved.

Sign in to view

Copy link
@SwamyDev

SwamyDev Dec 13, 2023

I find this a weird way to enforce an interface. Why not simply make this an abstract property and child classes need to provide a number:

@property
@abstractmethod
def num_coefficients(self):
    ...

Then just use that property everywhere else instead of _ncoefs. If the child class doesn't support or have interaction terms just return 0.

In case you need to different concepts of "number of coefficients" then you can introduce a second abstract property and use that instead. But if that's the case make sure both properties are named aptly so it's easy to distinguish the different concepts.

This comment was marked as resolved.

Sign in to view

Copy link
@raphaelquast

raphaelquast Feb 6, 2024

Author Contributor

this is done because ncoefs can be a fixed number for some functions (isotropic, rayleigh) or a number defined on init (e.g. in case a general form of the legendre expansion is known for arbitrary number of coefficients)

however, I agree that not hasattr(self, "_ncoefs") is obsolete now and can be removed since any volume- or surface scattering function should inherit from the base class that already sets _ncoefs to None if it is not explicitly provided.
raise AttributeError(
"You must specify the number of approximation coefficients for "
f"a {self.__class__.__name__} scattering function "
"in order calculate interaction-terms!"
)
return self._ncoefs

def scat_angle(self, t_0, t_ex, p_0, p_ex, a):
"""
Generalized scattering angle with respect to the given zenith-angles.
Expand Down
9 changes: 9 additions & 0 deletions src/rt1_model/plot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ def polarplot(
plotted on top of each other.
"""
if aprox is True:
try:
V_SRF.ncoefs
except Exception:
_log.warning(
"To print scatter function approximations, ncoefs must be provided!"
)
aprox = False

assert isinstance(inc, list), (
"Error: incidence-angles for " + "polarplot must be a list"
)
Expand Down
7 changes: 0 additions & 7 deletions src/rt1_model/surface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ def __init__(self, ncoefs=None, a=None):
self._ncoefs = ncoefs

assert len(self.a) == 3, "Generalization-parameter 'a' must contain 3 values"
assert self.ncoefs is not None, "Number of coefficients must be provided!"
assert self.ncoefs > 0, "Number of coeficients must be larger than 0!"

@property
def ncoefs(self):
"""The number of coefficients used in the legendre expansion."""
return self._ncoefs

def legcoefs(self):
"""Legendre coefficients of the BRDF."""
Expand Down
7 changes: 0 additions & 7 deletions src/rt1_model/volume.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ def __init__(self, ncoefs=None, a=None):
self._ncoefs = ncoefs

assert len(self.a) == 3, "Generalization-parameter 'a' must contain 3 values"
assert self.ncoefs is not None, "Number of coefficients must be provided!"
assert self.ncoefs > 0, "Number of coeficients must be larger than 0!"

@property
def ncoefs(self):
"""The number of coefficients used in the legendre expansion."""
return self._ncoefs

def legcoefs(self):
"""Legendre coefficients of the BRDF."""
Expand Down

0 comments on commit f312819

Please sign in to comment.