Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial rework of decorator warnings #515

Merged
merged 7 commits into from
Mar 5, 2025

Conversation

rg936672
Copy link
Contributor

@rg936672 rg936672 commented Feb 27, 2025

PR Type

  • Feature

Description

Decorators now check against a known "safe list" of overrides and new methods before raising a warning. This should reduce the number of spurious warnings that end users see. So far this has only been implemented for BinaryClassification above VariationalInference; expanding this to other decorators is left to future PRs.

Also improves the warning messages when they are raised, making it clearer exactly where the problem is coming from.

Progresses #123 but does not close it.

How Has This Been Tested?

Existing tests all pass. Added a temporary test to test_decorator_combinations to check that no spurious warnings are raised; this test should be incorporated into the general combinations test once implemented for all decorators.

Does this PR introduce a breaking change?

No.

Checklist before requesting a review

  • I have made sure that my PR is not a duplicate.
  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have performed a self-review of my code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@rg936672 rg936672 self-assigned this Feb 28, 2025
@gw265981 gw265981 requested a review from pc532627 March 4, 2025 09:36
Copy link
Contributor

@pc532627 pc532627 left a comment

Choose a reason for hiding this comment

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

@rg936672 Thanks for the changes, a few comments more for clarity rather than requesting code changes

@pc532627 pc532627 self-requested a review March 5, 2025 16:16
@pc532627 pc532627 merged commit b3b3023 into main Mar 5, 2025
36 checks passed
@pc532627 pc532627 deleted the feature/rework-decorator-warnings-prototype branch March 5, 2025 16:17
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