Skip to content

fix: stitching overlap with leading/trailing masked reflectivity values#211

Merged
glass-ships merged 14 commits intonextfrom
fix-stitching-overlap
Oct 8, 2025
Merged

fix: stitching overlap with leading/trailing masked reflectivity values#211
glass-ships merged 14 commits intonextfrom
fix-stitching-overlap

Conversation

@glass-ships
Copy link
Member

@glass-ships glass-ships commented Sep 30, 2025

Description of work:

  • remove deprecated stitch_reflectivity() method
  • rename smart_stitch_reflectivity() to stitch_reflectivity() for simplicity
  • update some type hinting
  • update pip audit vulnerability ignores

Check all that apply:
(if not, provide an explanation in the work description)

  • updated documentation and checked that it looks correct in the pull request preview
  • Source added/refactored
  • Added unit tests
  • Added integration tests

References:

  • Links to IBM EWM items: Defect 12742
  • Links to related issues or pull requests:

⚠️ Manual test for the reviewer

(instructions to set up the environment)

Check list for the reviewer

  • best software practices
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date and looks correct in the pull request preview
  • code comments added when explaining intent

@glass-ships glass-ships requested a review from Copilot October 2, 2025 16:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes stitching overlap issues with leading/trailing masked reflectivity values in neutron reflectometry data processing. The main change involves renaming the smart_stitch_reflectivity function to stitch_reflectivity and implementing better handling of masked data points during workspace preparation for stitching operations.

  • Renamed smart_stitch_reflectivity to stitch_reflectivity for clarity
  • Added logic to skip leading and trailing masked reflectivity values during stitching
  • Updated function calls and test references to use the new function name

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/quicknxs/interfaces/data_handling/data_manipulation.py Renamed function and added masked data handling in workspace preparation
src/quicknxs/interfaces/data_manager.py Updated function call and type annotation
src/quicknxs/interfaces/data_handling/instrument.py Added type annotations to class attributes
test/unit/quicknxs/interfaces/test_data_manager.py Updated test calls with q_cutoff parameter
test/unit/quicknxs/interfaces/data_handling/test_data_manipulation.py Updated test function names and calls
pyproject.toml Updated pip version and added vulnerability ignore

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.97%. Comparing base (df43ebc) to head (55dc31c).
⚠️ Report is 3 commits behind head on next.

Files with missing lines Patch % Lines
...knxs/interfaces/data_handling/data_manipulation.py 90.90% 1 Missing ⚠️
src/quicknxs/interfaces/data_manager.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next     #211      +/-   ##
==========================================
+ Coverage   64.94%   64.97%   +0.03%     
==========================================
  Files          36       36              
  Lines        6838     6810      -28     
==========================================
- Hits         4441     4425      -16     
+ Misses       2397     2385      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@glass-ships glass-ships requested a review from backmari October 2, 2025 16:31
Copy link
Collaborator

@backmari backmari left a comment

Choose a reason for hiding this comment

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

The code coverage shows that the change in _prepare_workspace_for_stitching is not covered by testing. Could you please add a unit test for this function? A test of stitch_reflectivity with const-Q binning could be another option that would test that change.

Copy link
Collaborator

@backmari backmari left a comment

Choose a reason for hiding this comment

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

Looks good!

@glass-ships glass-ships merged commit dd2db02 into next Oct 8, 2025
5 checks passed
@glass-ships glass-ships deleted the fix-stitching-overlap branch October 8, 2025 14:23
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.

4 participants