-
Notifications
You must be signed in to change notification settings - Fork 221
More deprecation updates for version 0.103.0 #4033
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
The discussion on the meeting was "Never" but I think to be precise you could say in the next major version release. |
That works for me. Just wanted to confirm. |
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.
my review for this one never posted! Could you look at these two things to finalize this PR
@@ -374,6 +374,7 @@ def plot_agreement_matrix(study, ordered=True, case_keys=None, axs=None): | |||
return fig | |||
|
|||
|
|||
# what's the dperecation strategy for this function in general? |
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.
@samuelgarcia / @alejoe91 this is for you both!
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.
the benchmark module is new and in dev, so I think we can just remove the function in the next release
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.
yes we can remove this
@@ -190,6 +190,7 @@ def get_agreement_sorting(self, minimum_agreement_count=1, minimum_agreement_cou | |||
) | |||
return sorting | |||
|
|||
# strategy for this dep? |
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.
same here and just below. What is the strategy?
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 think this warning has been there for a long time :)
IMO we can remove the functions in 0.103.0 directly.. @samuelgarcia what do you think?
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.
+1
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.
ok
@@ -16,10 +16,14 @@ def run_peak_pipeline( | |||
folder=None, | |||
names=None, | |||
): | |||
# TODO remove this soon | |||
# TODO remove this soon. Will be removed in 0.104.0 |
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 think we can remove this now. Nody never used it.
I am OK with evrything + one that can be removed now |
Thanks @zm711! Let's merge :) |
MockWaveformExtractor
and indicated it could happen in the futurereturn_scaled
we discussed a longer deprecation than 2 versions but right now it just says "future" just wanted this on our radar and explain why I didn't touch those. Maybe we want to discuss at some point what a long deprecation means for us? Or leave it as future like for WaveformExtractorI didn't double check testing so I assume this will break tests. Will fix tomorrow as I need to run.