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

Allow customized gone_in sentence for deprecation messages #10165

Closed
maresb opened this issue Jul 16, 2021 · 10 comments · Fixed by #10167
Closed

Allow customized gone_in sentence for deprecation messages #10165

maresb opened this issue Jul 16, 2021 · 10 comments · Fixed by #10167
Labels
type: enhancement Improvements to functionality type: maintenance Related to Development and Maintenance Processes

Comments

@maresb
Copy link
Contributor

maresb commented Jul 16, 2021

What's the problem this feature will solve?
The current pip._internal.utils.deprecation:deprecated function, in at least one case (#10128), leads to an awkward and confusing deprecation message. I would like to make the message more customizable so that awkward wording is reduced.

Describe the solution you'd like
See #10129 (comment). Namely, I would like to introduce a new kwarg gone_in_message with default value DEPRECATION_GONE_IN_MESSAGE = "pip {} will remove support for this functionality.". I would also like to be able to set gone_in_message="" to omit the automatic generation of such a message.

This would allow me to achieve my desired wording:

pip currently copies the source tree into a temporary directory before building it. pip 21.3 will build packages in-place within the original source tree ("in-tree build"). This new behavior can be tested with the --use-feature=in-tree-build flag. Future versions of pip will remove support for out-of-tree builds, and for this testing flag. You can find discussion regarding this at {link}.

Alternative Solutions
I have tried to reword things to work with the builtin message phrasing, but it forced me to use awkward sentences like "Regarding...". See #10129 for further discussion.

@maresb
Copy link
Contributor Author

maresb commented Jul 16, 2021

(I have prepared a commit in order to make a PR, but my current wifi is blocking port 22 so I can't push yet...)

@uranusjr
Copy link
Member

FWIW you can push via HTTPS (port 443). I use this often when I need to use public wifi 🙂

@pradyunsg pradyunsg added type: enhancement Improvements to functionality type: maintenance Related to Development and Maintenance Processes labels Jul 20, 2021
@pradyunsg
Copy link
Member

pradyunsg commented Jul 20, 2021

Part of the point of the whole deprecated function is to allow the author to avoid thinking about the specific wording -- so that it's standardised and we don't spend time on iterating that when doing deprecations.

My preference is that we update the default wording we use for gone_in, to something like "In pip {version}, this behaviour change will be enforced." or something broader like that, instead of making this configurable.

In other words, I'm a -1 on adding an argument for making this an argument and making this possible. I'm a +1 to changing this to help us avoid weird sentences in the future.

@pradyunsg
Copy link
Member

From #10129 (comment):

Some part of me wants to change it to something like:

DEPRECATION: pip copied a local directory before performing a build. This behaviour will change to stop copying and performing a build "in-tree".
pip 21.3 will enforce this behaviour change. You can use --use-feature=in-tree-build to test the upcoming behaviour.
Discussion regarding this change is at {link}.

This would, however, be a two-fold change -- partly here and partly in the deprecated function.

@maresb
Copy link
Contributor Author

maresb commented Jul 20, 2021

What bothers me about #10165 (comment) is that we are suggesting a flag which is already planned for deprecation. But based on the consensus from #10129 (comment) it seems that my concern is misguided.

So @pradyunsg, is your idea that you would like to add a new optional "preview" sentence like the following?

You can temporarily use --use-feature={preview_flag} to test the upcoming behavior.

@pradyunsg
Copy link
Member

pradyunsg commented Jul 23, 2021

So @pradyunsg, is your idea that you would like to add a new optional "preview" sentence like the following?

That + changing the "remove this functionality" language to "enforce this change" (specific wording is in the quoted comments).

@maresb
Copy link
Contributor Author

maresb commented Jul 23, 2021

Here is my suggested wording for #10129, sentence by sentence, in the proper order, and according to whether the behavior change is in the past or future. I will adjust #10167 accordingly. Please request any desired adjustments.

Reason sentence (future or past):

DEPRECATION: pip copied the source tree into a temporary directory before building it. This is changing so that packages are built in-place within the original source tree ("in-tree build").

Version sentence (future):

pip 21.3 will enforce this behavior change.

Version sentence (past):

This behavior change has been enforced since pip 21.3.

Preview sentence (future):

You can temporarily use the flag --use-feature=in-tree-build to test the upcoming behavior.

Preview sentence (past):
[empty]

Link sentence (future or past):

Discussion can be found at {link}.

@pradyunsg
Copy link
Member

I don't think we need any "past" phrasing. Once the default changes, we don't need to tell the users about it -- they'll either notice the change and deal with it; or be unaffected by the change.

@pradyunsg
Copy link
Member

The deprecated helper actually raises an exception, if you run it on a version that's equal or higher than gone_in.

@maresb
Copy link
Contributor Author

maresb commented Jul 27, 2021

I believe we need some sort of "past" phrasing in order to fix #10223. In any case, as you suggest, we should adjust the message (telling the users or not) based on whether the change is already implemented.

As you point out, the logic was already implemented for whether the message gets raised as a warning or an exception. Now #10167 adjusts the message in the appropriate way.

Now it is possible to do

deprecated(
    reason='pip copied the source tree into a temporary directory before building it. This is changing so that packages are built in-place within the original source tree ("in-tree build").',
    replacement=None,
    gone_in="21.3",
    feature_flag="in-tree-build",
    issue=7555,
)

When the change is upcoming (future), then the following warning occurs:

<stdin>:1: PipDeprecationWarning: DEPRECATION: pip copied the source tree into a temporary
directory before building it. This is changing so that packages are built in-place within
the original source tree ("in-tree build"). pip 21.3 will enforce this behavior change.
You can temporarily use the flag --use-feature=in-tree-build to test the upcoming behavior.
Discussion can be found at https://github.com/pypa/pip/issues/7555.

When the change has already taken place, the following exception occurs:

pip._internal.utils.deprecation.PipDeprecationWarning: DEPRECATION: pip copied the source
tree into a temporary directory before building it. This is changing so that packages are
built in-place within the original source tree ("in-tree build"). This behavior change has
been enforced since pip 21.3. Discussion can be found at
https://github.com/pypa/pip/issues/7555.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Improvements to functionality type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants