Skip to content

bpo-44388: Update venv EnvBuilder.ensure_directories() docs. #26663

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 2, 2021
Merged

bpo-44388: Update venv EnvBuilder.ensure_directories() docs. #26663

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b7373d
Update venv.ensure_directories() docs.
itsayellow Jun 10, 2021
1741783
Rephrase last sentence.
itsayellow Jun 15, 2021
5e66ecb
Clarify "this" and remove superfluous comma.
itsayellow Jun 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Doc/library/venv.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,11 @@ creation according to their needs, the :class:`EnvBuilder` class.

.. method:: ensure_directories(env_dir)

Creates the environment directory and all necessary directories, and
returns a context object. This is just a holder for attributes (such as
paths), for use by the other methods. The directories are allowed to
exist already, as long as either ``clear`` or ``upgrade`` were
specified to allow operating on an existing environment directory.
Creates the environment directory and all necessary subdirectories that
don't already exist, and returns a context object. This is just a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're updating this paragraph, another minor improvement to clarity may be as follows, because otherwise it's not very clear what 'this' refers to (e.g. the method, the instance, ..?). To me it's easier to understand if it's rephrased:

Suggested change
don't already exist, and returns a context object. This is just a
don't already exist, and returns a context object, which is just a

Copy link
Contributor Author

@itsayellow itsayellow Jun 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only hesitation I have with this change is that the sentence gets pretty long with a bunch of commas / clauses.

Another simple fix which fix would just be to change This is just to This context object is just. A little explicit, but I like the fact that it results in two sentences with 2 comma-separated clauses instead of one long sentence with 4 comma-separated clauses.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that was another option I've thought about. Both sound fine to me so if you prefer to keep 2 sentences, go ahead with that!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the two-sentence version and removed a comma after the parenthetical phrase that seemed superfluous to me.

holder for attributes (such as paths), for use by the other methods.
Any existing environment directories will remain unaffected as long as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest changing the sentence to:

If the :class:EnvBuilder is created with the arg clear=True, contents of the environment directory will be cleared and then all necessary subdirectories will be recreated.

-- this describes the effect of the method more fully and given the description, there is no reason for the reader to think that existing env dir and subdirs will be affected in any way.

Copy link
Contributor Author

@itsayellow itsayellow Jun 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that. 👍

I was erring on the side of explicitness, because this function looks like it has ability to write to the directory structure, and the thought of that worried me. If it were a purely information-gathering API function I think I would've left that part of the sentence out.

``clear`` was not specified.

.. method:: create_configuration(context)

Expand Down