-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[ENH] SliderGraph: thicker threshold line #7091
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7091 +/- ##
=======================================
Coverage 88.80% 88.80%
=======================================
Files 335 335
Lines 73916 73928 +12
=======================================
+ Hits 65638 65655 +17
+ Misses 8278 8273 -5 🚀 New features to boost your workflow:
|
Orange/widgets/utils/slidergraph.py
Outdated
| ev : HoverEvent | ||
| The hover event from pyqtgraph | ||
| """ | ||
| if not hasattr(ev, 'isEnter'): # Add check for event type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this check ever fail? Also, docstring declares this is a HoverEvent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you really need to check this, use if not isinstance(ev, HoverEvent).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Orange/widgets/utils/slidergraph.py
Outdated
| self._line.setPen(mkPen(self.palette().text().color(), width=2)) | ||
|
|
||
| # Set up hover state handling | ||
| self._line.hoverEvent = self._handle_hover # Changed from lambda to direct method reference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand the comment.
Also, I'd prefer deriving a new class from InfiniteLine and override the hoverEvent method. Monkey-patching is not very pythonic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, new class introduced
Orange/widgets/utils/slidergraph.py
Outdated
| # Set up hover state handling | ||
| self._line.hoverEvent = self._handle_hover # Changed from lambda to direct method reference | ||
|
|
||
| # Initial pen style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
ed44dc4 to
dd2d1f6
Compare
dd2d1f6 to
bfb7c28
Compare
ddf389d to
42a096c
Compare
feed758 to
df5114a
Compare
Issue
The vertical bar in the graphs such as scree plot (PCA) is almost invisible on the computer screen, and invisible when using a projector:
Description of changes
The bar is now thicker, and also more prominent (thick blue) when hovering.
Includes