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

🐛 Add Warning for VahadaneExtractor Algorithm Instability #871

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

mostafajahanifar
Copy link
Collaborator

This PR adds a warning to the VahadaneExtractor class to inform users about the algorithm's instability due to changes in the dictionary learning algorithm in scikit-learn versions > 0.23.0 (see issue #382). The docstrings are updated accordingly to reflect this warning. No other functionality is altered.

@shaneahmed shaneahmed changed the title BUG: Add warning for VahadaneExtractor algorithm instability :BUG: Add warning for VahadaneExtractor algorithm instability Oct 15, 2024
@shaneahmed shaneahmed changed the title :BUG: Add warning for VahadaneExtractor algorithm instability 🐛 Add Warning for VahadaneExtractor Algorithm Instability Oct 15, 2024
@shaneahmed shaneahmed added this to the Release v1.5.2 milestone Oct 18, 2024
@shaneahmed shaneahmed added the documentation Improvements or additions to documentation label Oct 18, 2024
@shaneahmed shaneahmed linked an issue Oct 18, 2024 that may be closed by this pull request
@@ -259,6 +266,14 @@ def __init__(
regularizer: float = 0.1,
) -> None:
"""Initialize :class:`VahadaneExtractor`."""
# Issue a warning about the algorithm's stability
logger.warning(
Copy link
Member

Choose a reason for hiding this comment

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

@mostafajahanifar Please can you add links to alternatives as discussed during the meeting?

Copy link
Member

@shaneahmed shaneahmed left a comment

Choose a reason for hiding this comment

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

Thanks @mostafajahanifar. Please can you add links to alternative repos users can use. Also, please can you add test using pytest capture caplog.text to verify the warning is caught?

Copy link

codecov bot commented Oct 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.88%. Comparing base (0b857c7) to head (86886d9).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #871   +/-   ##
========================================
  Coverage    99.88%   99.88%           
========================================
  Files           69       69           
  Lines         8656     8658    +2     
  Branches      1147     1147           
========================================
+ Hits          8646     8648    +2     
  Misses           4        4           
  Partials         6        6           

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vahadane stain-norm fails
2 participants