Skip to content

Conversation

@jaketanderson
Copy link
Collaborator

Addresses #218

Removed instances of "percentage" from the fraction entries in attach,
pull, release phase's descriptions inside DAT_restraint. Added checks
for calc method 3, which will now error for fraction vals outside [0,1].
@jaketanderson jaketanderson requested a review from Copilot July 16, 2025 23:42
Copy link

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

Clarify fraction-related parameters in documentation and enforce valid ranges for fraction_list in computation methods.

  • Updated docstrings for attach, pull, and release to describe unitless fraction parameters.
  • Added validation checks for fraction_list values outside [0,1] in _calc_method.
  • Logged errors and raised exceptions when fraction_list contains invalid entries.
Comments suppressed due to low confidence (1)

paprika/restraints/restraints.py:419

  • Add unit tests to cover cases where fraction_list contains values outside [0,1] to ensure this new validation logic is exercised.
            if not numpy.all(

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jeff231li
Copy link
Collaborator

@jaketanderson is this PR up-to-date and ready for review?

@jaketanderson
Copy link
Collaborator Author

@jaketanderson is this PR up-to-date and ready for review?

@jeff231li Yes, it should be ready. The main things that need to be reviewed are the rewritten descriptions of the fields and the new logic checking if someone is (incorrectly) using fraction_list as a percentage list. Please let me know if there are any issues.

@jeff231li
Copy link
Collaborator

Hi @jaketanderson, when you get the chance, can you add a unit test in this file to assert that the new update is behaving as it should be?

def test_DAT_restraint():

@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.93%. Comparing base (f0b31dc) to head (1b9290b).
⚠️ Report is 5 commits behind head on master.

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

@jaketanderson
Copy link
Collaborator Author

I added unit tests and fixed a major issue which was caught by them. The ci tests are still failing on python <= 3.12 due to unrelated numpy compatibility issues, and the Codacy scan is failing because more setup needs to be done on the part of someone with admin access to the GilsonLabUCSD account and I hesitate to ask for what could be a tedious account setup and API key generation process.

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