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

deprecate old-style functions #276

Open
mathause opened this issue Sep 4, 2023 · 6 comments
Open

deprecate old-style functions #276

mathause opened this issue Sep 4, 2023 · 6 comments
Milestone

Comments

@mathause
Copy link
Member

mathause commented Sep 4, 2023

We should raise deprecations warnings (well FutureWarning...) for the legacy functions. I am not 100% sure if already for 0.9 or only in 1.0. Also ensure the warnings are not raised 100 times...

@mathause mathause added this to the v0.9.0 milestone Sep 4, 2023
@mathause
Copy link
Member Author

Moving this to the v0.10.0 milestone

@mathause
Copy link
Member Author

copied from #599 (comment) by @veni-vidi-vici-dormivi

Yes agree to deprecate the rest - I am not sure what the best approach is, though - maybe a decorator?

Hm as far as is see the decorator needs a new dependency?

@mathause
Copy link
Member Author

I don't think so - or what did you have in mind? I thought along the lines of

from functools import wraps
import warnings

def deprecate(func):

    @wraps(func)
    def wrapped(*args, **kwargs):
        warnings.warn(
            "This mesmer code path is deprecated. Please use the new api. See example"
            " at XXX"
            FutureWarning
        )

        func(*args, **kwargs)

    return wrapped

We could also take a look at https://github.com/matplotlib/matplotlib/blob/main/lib/matplotlib/_api/deprecation.py but that is probably overkill

@veni-vidi-vici-dormivi
Copy link
Collaborator

Ah I thought you meant https://deprecated.readthedocs.io/en/latest/introduction.html

But your solution seems to do the same thing.
But, this would mean all the functions are still there right? So also the tests have to stay. (I was hoping we could get rid of some of the old test data, but its also fine if not)

@mathause
Copy link
Member Author

Ah didn't know about this package. It seems to be a similar approach than what I suggested (a bit more fancy and also for classes and awaitable functions etc.). We can also just remove the old code path. Or remove the tests and exclude them from the coverage (e.g. in combination with #262). We could also just raise a DeprecationError if one of the old style functions are imported or used. One thing I want to check before we do: did we do any changes to the old code path since the last release. Or yet another solution: make a release, remove the old style functions and do another release quickly after that (only question: which one is v1.0?)

Matlab also has a bunch of helper classes https://github.com/matplotlib/matplotlib/blob/main/lib/matplotlib/_api/deprecation.py where we could draw inspiration from (but probably also not necessary).

@veni-vidi-vici-dormivi
Copy link
Collaborator

So the only thing we changes is the switch from averaging standard deviations to averaging variances.

I would prefer raising Deprecation Errors and only releasing once I think. I see how the patch of averaging the variances in the old code path would not be in any release... but it is in the new code path so I would say this is fine?

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

No branches or pull requests

2 participants