Skip to content

Conversation

@Sparks29032
Copy link
Collaborator

Keeps feature of #259 we want per the discussion (see last comment).

@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.93%. Comparing base (f5fc9ac) to head (c791189).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #272   +/-   ##
=======================================
  Coverage   99.92%   99.93%           
=======================================
  Files          24       24           
  Lines        1398     1446   +48     
=======================================
+ Hits         1397     1445   +48     
  Misses          1        1           
Files with missing lines Coverage Δ
tests/test_morphsqueeze.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

This looks good. Please see comments.

I don't love the name ``--check-increases". It isn't checking anything.

How about --force-monotonic-x for example?

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

please see comments

@pytest.mark.parametrize(
"squeeze_coeffs, x_morph",
[
# The following squeezes make the function non-monotonic
Copy link
Contributor

Choose a reason for hiding this comment

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

# The following squeezes make the function non-monotonic. Expect code to work but issue the correct warning.

# call in .py
x_target = x_morph
y_target = np.sin(x_target)
coeffs = [squeeze_coeffs[f"a{i}"] for i in range(len(squeeze_coeffs))]
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason you don't just give as "squeeze_coeffs" inputs [-1, -1, 0, 0, 2] and remove this line? I think it would be easier to read. Less cognifive overload for the poor reviewer....

y_morph = np.sin(x_squeezed)
morph = MorphSqueeze()
morph.squeeze = squeeze_coeffs
with pytest.warns() as w:
Copy link
Contributor

Choose a reason for hiding this comment

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

w -> warning?

@sbillinge sbillinge merged commit 561499b into diffpy:main Dec 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants