Skip to content

Conversation

@jamesbraza
Copy link
Collaborator

Realized I didn't open source a unit test that I could have

@jamesbraza jamesbraza self-assigned this Jun 16, 2025
Copilot AI review requested due to automatic review settings June 16, 2025 23:13
@jamesbraza jamesbraza added the enhancement New feature or request label Jun 16, 2025
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

This PR adds a missing unit test for the oracle_solubility_eval function, ensuring its behavior is validated across multiple scenarios.

  • Imported the new oracle_solubility_eval function into the test suite
  • Added a parameterized test_oracle_solubility_eval covering diverse solubility cases
Comments suppressed due to low confidence (1)

tests/test_rewards.py:361

  • It may be valuable to add an assertion on metadata after calling oracle_solubility_eval to ensure expected metadata entries are populated.
    metadata: dict[str, JsonValue] = {}

), f"Reason for failure: {metadata}"


@pytest.mark.parametrize(
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] This long list of parameter cases could be refactored into a helper or fixture to reduce duplication and make individual cases easier to manage.

Copilot uses AI. Check for mistakes.
),
pytest.param(
"CCCCCC=CCCCN(C)CCC",
'("groups", ["cis double bond", "hetero N basic H"], -4.693881511688232, "decrease")', # noqa: E501
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] There are two spaces before the numeric literal; consider normalizing whitespace to a single space for consistency.

Suggested change
'("groups", ["cis double bond", "hetero N basic H"], -4.693881511688232, "decrease")', # noqa: E501
'("groups", ["cis double bond", "hetero N basic H"], -4.693881511688232, "decrease")', # noqa: E501

Copilot uses AI. Check for mistakes.
@jamesbraza
Copy link
Collaborator Author

Nevermind, I realized I put this test in packages/remotes/tests/test_rewards.py. So it was opened already, closing this out

@jamesbraza jamesbraza closed this Jun 16, 2025
@jamesbraza jamesbraza deleted the adding-missing-open-source-stuff branch June 16, 2025 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants