Skip to content

Improves invariant calculator tests#3897

Open
jellybean2004 wants to merge 21 commits intomainfrom
inv_tests
Open

Improves invariant calculator tests#3897
jellybean2004 wants to merge 21 commits intomainfrom
inv_tests

Conversation

@jellybean2004
Copy link
Copy Markdown
Member

Description

This PR refactors and expands the sasinvariant test suite for clarity, consistency, and branch coverage.

  • Migrated utest_* modules to pytest-style test modules.
  • Added focused tests for:
    • uncertainty propagation (closed-form and hard-coded checks),
    • extrapolation behavior (low/high/both),
    • invalid input handling and error branches,
    • mapper forwarding behavior via dedicated wrapper tests.
  • Added shared fixtures in conftest.py.

Coverage impact:

  • invariant.py coverage increased from 85% to ~99%.
  • invariant_mapper.py coverage increased from 0% to 100%.

How Has This Been Tested?

Ran the tests from the sasinvariant directory and checked the coverage report:

>>> pytest test/sasinvariant/ --cov=src/sas/sascalc/invariant --cov-report=html
...
============================= 114 passed in 2.32s =============================

Review Checklist:

Documentation

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing

  • The introduced changes comply with SasView license (BSD 3-Clause)

Copy link
Copy Markdown

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 pull request modernizes the invariant test suite and refactors parts of the invariant implementation to improve validation, typing, and uncertainty handling.

Changes:

  • Replaced legacy unittest-style invariant tests with pytest-based equivalents and shared fixtures.
  • Added new uncertainty-propagation tests (closed-form checks + hardcoded “oracle” snapshots).
  • Refactored invariant.py with typing/ABCs, vectorized q* integration/uncertainty, and improved error handling/logging.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/sas/sascalc/invariant/invariant.py Refactors invariant core (typing/ABC, vectorized integration, logging, stricter validation).
test/sasinvariant/conftest.py Adds shared pytest fixtures for synthetic and real invariant datasets.
test/sasinvariant/CalculatorTest.py Pytest port + expanded calculator behavior/validation tests.
test/sasinvariant/LinearTest.py Pytest port for extrapolator/linearization tests + additional edge cases.
test/sasinvariant/ExtrapolationTest.py Pytest port for Guinier/power-law extrapolation behavior.
test/sasinvariant/UseCasesTest.py Pytest port of “usage perspective” scenarios using bundled data files.
test/sasinvariant/UncertaintyTest.py New closed-form and hardcoded-value uncertainty tests.
test/sasinvariant/InvariantMapperTest.py New forwarding/adapter tests for invariant_mapper.
test/sasinvariant/utest_use_cases.py Removed legacy unittest-based use case tests (superseded by pytest suite).
test/sasinvariant/utest_data_handling.py Removed legacy unittest-based data-handling tests (superseded by pytest suite).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +431 to +433
def background(self, value: float):
self._background = value
self._qstar = None
Comment on lines +440 to +442
def scale(self, value: float):
self._scale = value
self._qstar = None
Copy link
Copy Markdown
Contributor

@krzywon krzywon left a comment

Choose a reason for hiding this comment

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

Much improved! The switch to pytest is also welcome here. I don't think we have an issue, but switching all tests from unittest should be a goal to remove the dependency.

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.

3 participants