Deprecate --index/--no-index in pip-compile

[atugushev]

To avoid ambiguity of pip-sync --no-index.

Resolves #373.

Changelog-friendly one-liner: Deprecate --index/--no-index in favor of --emit-index-url/--no-emit-index-url options in pip-compile.

Contributor checklist

• Provided the tests for the changes.
• Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
• Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

[codecov]

Codecov Report

Merging #1130 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1130   +/-   ##
=======================================
  Coverage   99.49%   99.50%           
=======================================
  Files          36       36           
  Lines        2773     2802   +29     
  Branches      328      331    +3     
=======================================
+ Hits         2759     2788   +29     
  Misses          8        8           
  Partials        6        6           

Impacted Files	Coverage Δ
tests/test_utils.py	100.00% <ø> (ø)
piptools/scripts/compile.py	100.00% <100.00%> (ø)
piptools/writer.py	100.00% <100.00%> (ø)
tests/test_cli_compile.py	100.00% <100.00%> (ø)
tests/test_writer.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 008865a...1b3f496. Read the comment docs.

[atugushev]

I've just thought about the case when --index and --no-emit-index-url conflict to each other. Shouldn't we throw the error or prefer --no-emit-index-url?

[atugushev]

@hugovk @webknjaz

As a follow up of the twitter thread I wonder what do you think about the deprecation technique in this PR?

[hugovk]

Looks about right!

In our project, we decided not to use PendingDeprecationWarning first, and go straight to DeprecationWarning to increase visibility from the start.

[atugushev]

@AndydeCleyre

I wonder what would you think before we merge this?

I've just thought about the case when --index and --no-emit-index-url conflict to each other. Shouldn't we throw the error or prefer --no-emit-index-url?

Especially for this case.

[AndydeCleyre]

Nice work! Three notes/questions:

I've just thought about the case when --index and --no-emit-index-url conflict to each other. Shouldn't we throw the error or prefer --no-emit-index-url?

I say either throw an error, or prefer the right-most flag (not necessarily the not-deprecated one).

2. Slightly off topic: I don't like the click-style --flag/--no-flag options in general; is there any way for a user to know which is the default? It doesn't seem to be indicated in the --help output.

3. I think we only need one type of warning so far, and should change the name and base for it.

Why bother with a PipToolsPendingDeprecationWarning? You may be using the term "deprecation" to mean something closer to "unsupported." The docstring for PipToolsDeprecationWarning says:

Deprecates things in the next or a specific version.

but aren't we trying to warn about things that are currently deprecated, to be unsupported/removed in the future?

The Python docs describe their own PendingDeprecationWarning as:

Base class for warnings about features which are obsolete and expected to be deprecated in the future, but are not deprecated at the moment.

This class is rarely used as emitting a warning about a possible upcoming deprecation is unusual, and DeprecationWarning is preferred for already active deprecations.

Then the description of their DeprecationWarning makes me think that isn't the best analogue either:

Base class for warnings about deprecated features when those warnings are intended for other Python developers.

We are really targeting our users here, although the line between "users" and "Python developers" is not so obvious for a project like this.

I think what we're going for is a much better fit with Python's FutureWarning:

Base class for warnings about deprecated features when those warnings are intended for end users of applications that are written in Python.

Note that this is not ignored by default as the other two are.

Seeing this at the warnings docs made it clear that the conceptual classifications have changed pretty recently, making FutureWarning decidedly more appropriate here:

Changed in version 3.7: Previously DeprecationWarning and FutureWarning were distinguished based on whether a feature was being removed entirely or changing its behaviour. They are now distinguished based on their intended audience and the way they’re handled by the default warnings filters.

[atugushev]

@AndydeCleyre

Thanks for the analysis as always and sorry for the delayed answer. Regarding the docs:

FutureWarning — Base category for warnings about deprecated features when those warnings are intended for end users of applications that are written in Python.

 
It seems FutureWarning is our candidate since it's intended to warn end-users of the application and it's visible default so there is no need in setup_warning().

Slightly off topic: I don't like the click-style --flag/--no-flag options in general; is there any way for a user to know which is the default? It doesn't seem to be indicated in the --help output.

With show_default=True:

  --emit-trusted-host / --no-emit-trusted-host
                                  Add trusted host option to generated file
                                  [default: True]

[atugushev]

@AndydeCleyre

Refactor to FutureWarning addressed in 302f067.

I say either throw an error, or prefer the right-most flag (not necessarily the not-deprecated one).

Mutual exclusiv addressed in 3211b94.

But now there is a problem with get_compile_command() :( Need time to figure out how to handle flags with default=None. For --pre it's False, for --emit-index-url it's True. Huh. Don't really want to hard-code, but don't see other options for now...

[atugushev]

@AndydeCleyre

I came up with the solution acae7bf. What do you think?

[atugushev]

@AndydeCleyre kindly ping

[AndydeCleyre]

Looks good. As usual, I'm no Click expert, but if this fills the gap needed, 👍 .

Can you add a note to the has_args docstring about its behavior regarding presence of the negative counterparts to args (--no-...)?

Also still prefer "mutually exclusive"

[AndydeCleyre]

Excellent, thanks! All systems go, as far as I'm concerned.

[atugushev]

@AndydeCleyre

Thanks! I believe cli.has_arg(...) would help to refactor and simplify get_compile_command, and also to fix #811 where we need to track os args, so this change is definitely useful!