A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

No problems, thanks for the review.
I will revert the style things and apply your suggestions. I would love to come and talk to you about reformatting a lot of this module. Is there a goal that the stdlib is PEP8?

p.s. I am already in ACKs for a previous change. Thanks for asking.

in fact, there is no plan for the stdlib PEP8 compliant excepted for the new code, not for the former and there is a reason for that. if we migrate the stdlib to PEP8, we will have a lot of conflicts for a backport :/

for the new Python code, you can be PEP8 compliant.

Do you all need anything else from me? Would love to see this merge :)

Rebased my fork and force pushed the update so merge conflicts should go away.

@ambv: Please replace # with GH- in the commit message next time. Thanks!

Thanks! ✨ 🍰 ✨

It looks like this didn't land in 3.8.0. I was hoping the implementation could be improved before being released in any 3.8.x.

Personally, I'd never use python3 -m venv .venv --upgrade-deps as written because it will be significantly slower than writing my own shell alias to do python3 -m venv .venv --without-pip and then installing pip directly (say with get-pip.py or even just executing the bundled .whl file).

So, rather than install the bundled pip, and then use that pip installation to uninstall itself and install latest version of pip, wouldn't it be better just to plumb through to ensurepip directly and add ability to fetch latest from index in the first place?

Upsides:

• cleaner
• faster
• less convoluted

Downsides:
?

Pinging @ncoghlan and @vsajip - thoughts?

FWIW, this is the kind of shell function I had in mind:

function ve() {
    if [ ! -d ./.venv ]; then
        echo "creating venv..."
        if ! python3.8 -m venv .venv --prompt=$(basename $PWD) --without-pip; then
            echo "ERROR: Problem creating venv" >&2
            return 1
        else
            .venv/bin/python /usr/local/lib/python3.8/ensurepip/_bundled/pip-19.2.3-py2.py3-none-any.whl/pip install --upgrade pip setuptools
            source .venv/bin/activate
        fi
    else
        source .venv/bin/activate
    fi
}

Obviously you don't have to hardcode the path to the whl file, you can build it off of ensurepip._bundled.__path__ if you want (for now, importlib.resources doesn't make this easy because there is no __init__.py in the _bundled subdirectory for 3.8.0)

Thanks for looking into this. All for the more efficient way to avoid the double install and passing this into ensurepip is definitely the way, but I was not sure if it could handle remote fetches to PyPI if it was needed.

I admit I went the easier route that introduced less change so I had something as performance of the creation was not a high priority for me (and it's optional) and in scripts I was already effectively doing it.