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

Warn that at least one deprecation warning emitted, default depwarn=yes again #37164

Closed
BioTurboNick opened this issue Aug 23, 2020 · 17 comments · Fixed by #37296
Closed

Warn that at least one deprecation warning emitted, default depwarn=yes again #37164

BioTurboNick opened this issue Aug 23, 2020 · 17 comments · Fixed by #37296

Comments

@BioTurboNick
Copy link
Contributor

Based on the discussion here: #35362 and having run into this silent failure mode, I suggest a compromise:

  • Restore default to depwarn=yes
  • Add depwarn=verbose
  • Under yes, Julia tracks deprecations received but does not print them. On receipt, it just prints a single warning shown below, with details accessible by a function call.
  • Under verbose, all warnings are printed as they are received, which is the current behavior for yes
  • Under no, no deprecations are tracked.

The single warning under yes would be something like:

┌ Warning: The running code contains deprecations at REPL[1]:1.
│ Use `depwarns()` to print the list of warnings, or start Julia with --depwarns=verbose to show warnings when they happen.
│ Deprecated code will still execute correctly. This warning will not be displayed again during this session.
└ @ REPL[1]:1

This could even just be a boolean switch that gets flipped on for the first deprecation received and nothing else, just to inform the user that they can take an action to see the warnings. More usefully, it could store the deprecation messages received in a set and display the specific ones received when a function is called, as in the example message above.

I can understand that it might still bother users of big packages to see a warning at all, but I think the tradeoff would be fair?

@BioTurboNick BioTurboNick changed the title Warn that at least one deprecation warning emitted when depwarn=no Warn that at least one deprecation warning emitted, default depwarn=yes again Aug 23, 2020
@KristofferC
Copy link
Sponsor Member

Please provide information about the actual problem here instead of just linking to an issue with a lot of discussion. What silent failure?

@BioTurboNick
Copy link
Contributor Author

The actual problem is that warnings are useful and important information that should not be completely silent by default, or else they wouldn't be warnings. There must be a default pathway leading the user from ignorance to knowledge that doesn't involve hunting around or stumbling on a problem unawares.

@BioTurboNick
Copy link
Contributor Author

BioTurboNick commented Aug 23, 2020

I'm just suggesting an improvement that keeps the benefits of warnings while addressing the slowdowns and output clutter caused by tons of warnings being emitted.

@StefanKarpinski
Copy link
Sponsor Member

I think it may make sense to have soft & hard deprecations: soft ones are silent by default whereas hard ones warn by default. Then you can opt to silence all deprecations, print all deprecations and/or make deprecations fatal.

@StefanKarpinski
Copy link
Sponsor Member

The problem here is that some people (e.g. Base) want to introduce deprecations that are silent by default whereas other people, e.g. LightGraphs, want to use the same deprecation mechanisms to introduce deprecations that are on by default so that they can guide people to change their code while allowing it to keep running like we did pre-1.0.

@BioTurboNick
Copy link
Contributor Author

I see, thanks @StefanKarpinski.

Would I be correct in presuming that among Base there is a desire to avoid adding overhead without a critical reason, and tracking warnings would do that?

If so, I can certainly understand that. Then I would at least advocate for the minimal "there was at least one warning" warning. If a warning is generated, one boolean gets flipped to true, and one message is emitted the first time it flips. It's just enough information for someone to act, but (I presume?) adds very little in complexity.

@KristofferC
Copy link
Sponsor Member

The actual problem is that warnings are useful and important information that should not be completely silent by default, or else they wouldn't be warnings.

Eh, this is not universally true. Warnings tend to have an intended audience and for people outside that, they are often useless and annoying. For example, if one of your deep dependencies is using some deprecated API of another one you don't care.

@timholy
Copy link
Sponsor Member

timholy commented Aug 26, 2020

I've been running across this recently and even have a planned ugly hack to get around this (JuliaImages/ImageCore.jl#131 (comment)). The distinction I see is that some warnings really can be directed just at upstream packages, in which case the new default is very nice. But others apply to every user and any scripts they have sitting on their hard drives, and for that the new default is really bad.

I was thinking about adding a kwarg to Base.depwarn, could that allow us to get both the things we want?

@BioTurboNick
Copy link
Contributor Author

Warnings tend to have an intended audience and for people outside that, they are often useless and annoying. For example, if one of your deep dependencies is using some deprecated API of another one you don't care.

That's fair. Is there a way to distinguish the audiences? Perhaps warnings could have a scope, and they only surface by default from one level down?

E.g. if I do using StatsPlots in Main, I'll only get warnings that originate in StatsPlots and not one of its dependencies like Distributions, unless I then also do using Distributions.

@StefanKarpinski
Copy link
Sponsor Member

What about only showing warnings for the current project and dev'd dependencies? The former because that's what you're working on, the latter because if you have it dev'd then we might provoke you to fix it 😆

@kimikage
Copy link
Contributor

This is off-topic, but let me check on the terminology, to be sure, @timholy.

some warnings really can be directed just at upstream packages

By "upstream" do you mean the side which "uses" (i.e. depends on) a package? That expression is often consistent with functional abstraction (low-level / high-level), but in the context of dependency, I think it's the opposite. I'm sorry if I misunderstood. (In any case, it seems worth defining clearly in the Pkg documentation.)

@timholy
Copy link
Sponsor Member

timholy commented Aug 27, 2020

No, you're right, I got it backwards. I meant "downstream."

@KristofferC
Copy link
Sponsor Member

That's fair. Is there a way to distinguish the audiences?

To me, there is pretty much only one time where you want to see deprecation warnings and that is when you are about to update your dependencies and are in a position to fix them. In normal usage, during presentations, etc you definitely don't want to see them. Therefore, it makes sense to me that they are opt-in.

@timholy
Copy link
Sponsor Member

timholy commented Aug 27, 2020

Lots of things do cause the registry to update. Suppose I'm a user who said add PkgA, which uses PkgB but I didn't explicitly as for PkgB or control the version. Now PkgB adds a depwarn, and the conscientious developers of PkgA notice, fix the issue, and adjust their bounds to support either the old or new behavior. But if it turns out my scripts depend in some unanticipated way on PkgB, how am I to be notified?

Concrete example: ImageCore is deprecating some pirating convert methods and encouraging broadcasting instead. How do we get the word out before the old calls start doing different stuff? Most people get ImageCore via Images and don't know or care what version they have.

@KristofferC
Copy link
Sponsor Member

KristofferC commented Aug 27, 2020

But if it turns out my scripts depend in some unanticipated way on PkgB, how am I to be notified?

Haven't you already "lost" at this point? PkgB could very well have completely rewritten the package and released a new major version that is completely different (without adding any deprecation warnings) and then PkgA update to use it without breaking its own API. If you then rely on PkgB things won't work. If you depend on PkgB you should add it to your project.

@BioTurboNick
Copy link
Contributor Author

That's fair. Is there a way to distinguish the audiences?

To me, there is pretty much only one time where you want to see deprecation warnings and that is when you are about to update your dependencies and are in a position to fix them. In normal usage, during presentations, etc you definitely don't want to see them. Therefore, it makes sense to me that they are opt-in.

I think this works better for people who work with the language nearly full time. For most other people in normal usage, they just won't know there is even an option to see warnings, and will miss them until something bites them. Then they'll be wondering why they had no warning.

@timholy
Copy link
Sponsor Member

timholy commented Aug 27, 2020

Haven't you already "lost" at this point?

Not if the developers are conscientious about using depwarns for upcoming breaking changes.

Besides, it's really easy for all but the most fastidious user to not realize that convert(RGB, A) depends on some internal package. Especially since we did a naughty thing and greased the skids. (But we're not the only pirates out there.)

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

Successfully merging a pull request may close this issue.

5 participants