Skip to content

Fix initialization issue with single smooth tendency #18

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

Merged
SBlokhuizen merged 3 commits into from
May 26, 2025
Merged

Fix initialization issue with single smooth tendency #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec06eba
fix initialiation issue with smooth tendency
SBlokhuizen Feb 18, 2025
975a5fc
Merge branch 'develop' into bugfix/fix-single-smooth
SBlokhuizen Feb 18, 2025
2527eb9
Merge remote-tracking branch 'origin/develop' into bugfix/fix-single-…
SBlokhuizen Mar 4, 2025
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
14 changes: 13 additions & 1 deletion tests/tendencies/test_smooth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_prev_and_next_value():
assert not tendency.annotations


def test_generate():
def test_get_value():
"""
Check the generated values.
"""
Expand All @@ -83,3 +83,15 @@ def test_generate():
assert values[0] == 3
assert values[-1] == 6
assert not tendency.annotations


def test_get_value_no_start():
"""
Check the generated values when no start is provided.
"""
tendency = SmoothTendency(user_duration=8, user_from=3, user_to=6)
_, values = tendency.get_value()

assert values[0] == 3
assert values[-1] == 6
assert not tendency.annotations
8 changes: 8 additions & 0 deletions waveform_editor/tendencies/smooth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def __init__(self, **kwargs):
self.from_ = 0.0
self.to = 0.0
super().__init__(**kwargs)
self._update_values()
Copy link
Copy Markdown
Collaborator

@maarten-ic maarten-ic Feb 18, 2025

Choose a reason for hiding this comment

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

_update_values also has the on_init=True flag in the @depends decorator. Do you know why that is not sufficient?

Copy link
Copy Markdown
Collaborator Author

@SBlokhuizen SBlokhuizen Feb 18, 2025

Choose a reason for hiding this comment

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

Yes, the on_init=True will cause the method to be called when the __init__ of the param.Parametrized base class is called, namely here. Since we now set the parameters after the intialization, the method is not called again.

Alternatively, a more declarative way to handle this would be:

@depends(
    "_trigger",
    "from_",
    "to",
    "start",
    "end",
    "start_derivative",
    "end_derivative",
    watch=True,
    on_init=True,
)

At the expense of more triggers. We then should also account for the intermediate triggers (for example, CubicSpline will throw an error if start > end, which could occur if start is updated before end).


def get_value(
self, time: Optional[np.ndarray] = None
Expand All @@ -46,6 +47,12 @@ def get_value(
time = np.linspace(float(self.start), float(self.end), num_steps)

values = self.spline(time)

if np.any(np.isnan(values)):
raise ValueError(
"A spline was generated at a time outside of its generated time range."
)

return time, values

def get_derivative(self, time: np.ndarray) -> np.ndarray:
Expand Down Expand Up @@ -110,6 +117,7 @@ def _update_values(self):
[self.start, self.end],
[from_, to],
bc_type=((1, d_start), (1, d_end)),
extrapolate=False,
)

values_changed = (
Expand Down