Skip to content

/ cpython

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

bpo-39429: Add a new "Python Development Mode" doc page #18132

Merged
merged 5 commits into from Jan 24, 2020
Merged

bpo-39429: Add a new "Python Development Mode" doc page #18132

merged 5 commits into from Jan 24, 2020
+268 −49

Conversation

@vstinner
Copy link
Member

vstinner commented Jan 23, 2020 •
edited by bedevere-bot

https://bugs.python.org/issue39429
@vstinner
bpo-39429: Add a new "Python Development Mode" doc page
Loading status checks…
49c7bc3
@vstinner vstinner requested review from 1st1 and asvetlov as code owners Jan 23, 2020
@vstinner

This comment has been minimized.

Sign in to view
Copy link
Member Author

vstinner commented Jan 23, 2020

cc @pablogsal @JulienPalard
@vstinner vstinner added the skip news label Jan 23, 2020
@vstinner
Rephase warnings doc
Loading status checks…
5430c2d
@asvetlov
asvetlov approved these changes Jan 23, 2020
View changes
@aeros
aeros reviewed Jan 23, 2020
View changes
Copy link
Member

aeros left a comment •
edited

Here's some suggestions, mostly minor grammatical fixes. The line width on some of them may need to be adjusted.

I'll go over it again later when I get the chance, to see if I missed anything.
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated
* Add ``default`` :ref:`warning filter <describing-warning-filters>`, as
:option:`-W default <-W>` command line option. The following warnings are
shown, whereas they are ignored by the default warning filters:

* :exc:`DeprecationWarning`;
* :exc:`ImportWarning`;
* :exc:`PendingDeprecationWarning`;
* :exc:`ResourceWarning`.
Comment on lines 21 to 28

This comment has been minimized.

Sign in to view
Copy link
@aeros

aeros Jan 23, 2020 •
edited

Member

Hmm, the wording of this section isn't entirely clear to me. Is it explaining that the listed warnings are not filtered in development mode, so they're shown instead of hidden? Also, that to re-use the default warning filters, a new command-line option "-W default" has been added?

I had to read over this part a few times, and I'm still not 100% certain that I'm quite getting it.

If I'm correctly understanding it, I'd recommend restructuring it a bit to make that more clear (might need to adjust line width):

Normally, the following warnings are filtered by the default 
:ref:`warning filters <describing-warning-filters>`:

  * :exc:`DeprecationWarning`;
  * :exc:`ImportWarning`;
  * :exc:`PendingDeprecationWarning`;
  * :exc:`ResourceWarning`.

In development mode, these warnings are no longer filtered. To re-enable the default
warning filters, use the new :option:`-W default <-W>` command line option.

(Correct me if I'm misunderstanding the section)

This comment has been minimized.

Sign in to view
Copy link
@vstinner

vstinner Jan 23, 2020

Author Member

I rewrote this section.
Doc/library/devmode.rst Outdated

* Enable :ref:`asyncio debug mode <asyncio-debug-mode>`. Similar to setting
:envvar:`PYTHONASYNCIODEBUG` environment variable to ``1``.
* Check *encoding* and *errors* arguments on string encoding and decoding

This comment has been minimized.

Sign in to view
Copy link
@aeros

aeros Jan 23, 2020

Member
Suggested change
* Check *encoding* and *errors* arguments on string encoding and decoding
* Check the *encoding* and *errors* arguments for string encoding and decoding
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/sys.rst Outdated Show resolved Hide resolved
Include/cpython/initconfig.h Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/c-api/init_config.rst Show resolved Hide resolved
@aeros

This comment has been minimized.

Sign in to view
Copy link
Member

aeros commented Jan 23, 2020 •
edited

Also, thanks for adding this highly informative section, @vstinner! I actually learned a decent amount about the development mode that I wasn't previously aware of while proofreading it.
@csabella
csabella reviewed Jan 23, 2020
View changes
Doc/library/devmode.rst Outdated

The Python Development Mode introduces additional runtime checks which are too
expensive to be enabled by default. It should not be more verbose than the
default if the code is correct: new warnings are only emitted when an issue is

This comment has been minimized.

Sign in to view
Copy link
@csabella

csabella Jan 23, 2020

Contributor

The colon should probably be a semi-colon.
@csabella
csabella reviewed Jan 23, 2020
View changes
Doc/library/devmode.rst Outdated Show resolved Hide resolved
@csabella
csabella reviewed Jan 23, 2020
View changes
Doc/library/devmode.rst Outdated Show resolved Hide resolved
@csabella

This comment has been minimized.

Sign in to view
Copy link
Contributor

csabella commented Jan 23, 2020 •
edited

This is amazing. Thank you! Big +1.
@vstinner
Address reviews comments
Loading status checks…
3cca31c
@vstinner
vstinner reviewed Jan 23, 2020
View changes
Copy link
Member Author

vstinner left a comment

@aeros @csabella: I tried to address all comments, but I also made a few more minor changes. Would you mind to review my PR again?
Doc/library/devmode.rst Outdated
* Add ``default`` :ref:`warning filter <describing-warning-filters>`, as
:option:`-W default <-W>` command line option. The following warnings are
shown, whereas they are ignored by the default warning filters:

* :exc:`DeprecationWarning`;
* :exc:`ImportWarning`;
* :exc:`PendingDeprecationWarning`;
* :exc:`ResourceWarning`.

This comment has been minimized.

Sign in to view
Copy link
@vstinner

vstinner Jan 23, 2020

Author Member

I rewrote this section.
Doc/library/devmode.rst Outdated Show resolved Hide resolved
@aeros
aeros reviewed Jan 23, 2020
View changes
Copy link
Member

aeros left a comment •
edited

I tried to address all comments, but I also made a few more minor changes. Would you mind to review my PR again?

No problem. It looks significantly improved with the latest round of changes. Here's a few additional suggestions (some from new areas, others from existing ones I didn't see before):
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
Doc/library/devmode.rst Outdated Show resolved Hide resolved
@vstinner
Address aeros's review
Loading status checks…
18a22c2
@aeros
aeros approved these changes Jan 23, 2020
View changes
Copy link
Member

aeros left a comment •
edited

One last remaining change in L92-L93 of Doc/library/devmode.rst:

The Python Development Mode does not prevent the :option:`-O` command line option to
remove :keyword:`assert` statements nor to set :const:`__debug__` to ``False``.

to

The Python Development Mode does not prevent the :option:`-O` command line
option from removing :keyword:`assert` statements nor from setting
:const:`__debug__` to ``False``.

(Grammar and line width fix)

Other than that, LGTM! Thanks @vstinner.
@vstinner
prevent to remove => prevent from removing
Loading status checks…
a13d8e6 
Truncate also long lines
@vstinner

This comment has been minimized.

Sign in to view
Copy link
Member Author

vstinner commented Jan 24, 2020

One last remaining change in L92-L93 of Doc/library/devmode.rst: (...) (Grammar and line width fix)

fixed
@vstinner vstinner merged commit b9783d2 into python:master Jan 24, 2020
11 checks passed
11 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200124.19 succeeded
Details
bedevere/issue-number Issue number 39429 found
Details
bedevere/news "skip news" label found
codecov/patch Coverage not affected when comparing 41f0ef6...a13d8e6
Details
codecov/project 83.17% (+0.99%) compared to 41f0ef6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner vstinner deleted the vstinner:devmode_doc branch Jan 24, 2020
@vstinner

This comment has been minimized.

Sign in to view
Copy link
Member Author

vstinner commented Jan 24, 2020

Thanks @aeros and @csabella for your reviews!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csabella csabella

@asvetlov asvetlov

@aeros aeros

@1st1 1st1

Assignees
No one assigned
Labels
CLA signed skip news
Projects
None yet
Milestone
No milestone
6 participants
@vstinner @aeros @csabella @asvetlov @the-knights-who-say-ni @bedevere-bot
You can’t perform that action at this time.