Simplify using CurveItem subclasses in Plot subclasses #1042
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This mirrors for all the other Plot classes what was already done with PyDMTimePlot: Have a `createCurveItem` method that returns the relevant object for that plot, which can be overriden in the plot's subclasses for customized CurveItem implementations. Signed-off-by: flowln <thiago.ferreira@lnls.br>
nstelter-slac
requested review from
nstelter-slac
and removed request for
nstelter-slac
|
We agree, the change to pass a generic *args, **kwargs would be beneficial. Would you like to implement that with this PR? |
This gives more flexibility to the subclasses to change their signatures of __init__. Signed-off-by: flowln <thiago.ferreira@lnls.br>
There was a problem hiding this comment.
Thanks! Testing this PR did uncover a mistake I made a while back without noticing where I misnamed a keyword argument as y_channel. Would you mind changing line 522 of timeplot.py to
channel_address=y_channel,
Unfortunately github doesn't let me make suggested changes on lines that weren't edited.
Signed-off-by: flowln <thaigo.ferreira@lnls.br>
Signed-off-by: flowln <thiago.ferreira@lnls.br>
flowln
force-pushed
the
improve_plots_subclassing
branch
from
fc2c2be
to
6aff422
Compare
jbellister-slac
approved these changes
Oct 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While making subclasses for TimePlot and WaveformPlot for my application, I noticed this little inconvenience with non-TimePlot plots. I think it's a good thing to have a factory method such as this in every plot! 🙂
I kept it as close as possible to the one in TimePlot, so as to not change more stuff than needed, though I figured it may be even better to pass a generic
*args, **kwargsto thecreateCurveItemfunctions, so as to not constrain the signature of the CurveItem custom implementations. What do you think?