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

[docs] Minor information-ordering issue in __main__ doc #90437

Closed
ferdnyc mannequin opened this issue Jan 6, 2022 · 12 comments · May be fixed by #30480
Closed

[docs] Minor information-ordering issue in __main__ doc #90437

ferdnyc mannequin opened this issue Jan 6, 2022 · 12 comments · May be fixed by #30480
Labels
3.11 only security fixes docs Documentation in the Doc dir easy type-bug An unexpected behavior, bug, or error

Comments

@ferdnyc
Copy link
Mannequin

ferdnyc mannequin commented Jan 6, 2022

BPO 46279
Nosy @taleinat, @merwok, @ferdnyc, @riqts
PRs
  • gh-90437: Fix __main__.py documentation wording #30480
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2022-01-06.12:57:08.792>
    labels = ['easy', '3.11', 'type-bug', 'docs']
    title = '[docs] Minor information-ordering issue in __main__ doc'
    updated_at = <Date 2022-01-13.09:18:22.891>
    user = 'https://github.com/ferdnyc'

    bugs.python.org fields:

    activity = <Date 2022-01-13.09:18:22.891>
    actor = 'AlexWaygood'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2022-01-06.12:57:08.792>
    creator = 'ferdnyc'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46279
    keywords = ['patch', 'easy']
    message_count = 9.0
    messages = ['409839', '409997', '410085', '410087', '410088', '410091', '410093', '410121', '410461']
    nosy_count = 5.0
    nosy_names = ['taleinat', 'eric.araujo', 'docs@python', 'ferdnyc', 'jacksonbrummell1']
    pr_nums = ['30480']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue46279'
    versions = ['Python 3.11']

    Linked PRs

    @ferdnyc
    Copy link
    Mannequin Author

    ferdnyc mannequin commented Jan 6, 2022

    The expanded documentation on top-level environments is quite an improvement, but there's one passage that causes some confusion. In the section '__main__.py in Python Packages', towards the end, it reads:

    """
    This won’t work for __main__.py files in the root directory of a .zip file though. Hence, for consistency, minimal __main__.py like the venv one mentioned above are preferred.
    """

    Problem is, that's the first mention of venv anywhere in the document. There's a 'See also:' box right BELOW the sentence in question that references venv and its minimal __main__.py, but it hasn't been introduced yet when that first mention comes along.

    @ferdnyc ferdnyc mannequin added the 3.11 only security fixes label Jan 6, 2022
    @ferdnyc ferdnyc mannequin assigned docspython Jan 6, 2022
    @ferdnyc ferdnyc mannequin added the docs Documentation in the Doc dir label Jan 6, 2022
    @merwok
    Copy link
    Member

    merwok commented Jan 7, 2022

    Do you have a suggestion on how to fix this?

    @riqts
    Copy link
    Mannequin

    riqts mannequin commented Jan 8, 2022

    It seems this is a simple miswording, where the word 'above' is used when the see below example clarifies BELOW the text.

    I will create a pull request within the hour

    @riqts
    Copy link
    Mannequin

    riqts mannequin commented Jan 8, 2022

    Updating issue, Have submitted fix

    @ferdnyc
    Copy link
    Mannequin Author

    ferdnyc mannequin commented Jan 8, 2022

    TBH, personally I don't think I'd just reword it with "below". That seems like the path of least resistance, but then the sentence becomes this:

    """
    This won’t work for __main__.py files in the root directory of a .zip file though. Hence, for consistency, minimal __main__.py like the venv one mentioned below are preferred.
    """

    Doesn't really track. How can you draw conclusions ("Hence...") from something that hasn't even been discussed yet?

    It might actually be better to just drop the mention of venv from that particular sentence. The see-also text introduces *itself* as an example of what was just discussed. There's no real reason to bring it up ahead of time, because the see-also already flows naturally from the previous discussion without any setup.

    @taleinat
    Copy link
    Contributor

    taleinat commented Jan 8, 2022

    I agree that it seems better to avoid menntioning venv in that sentence. Specifically I suggest:

    This won’t work for __main__.py files in the root directory of a .zip file though. Hence, for consistency, minimal __main__.py without a __name__ check are preferred.

    @ferdnyc
    Copy link
    Mannequin Author

    ferdnyc mannequin commented Jan 8, 2022

    Maybe,

    """
    This won’t work for main.py files in the root directory of a .zip file though. Thus, for consistency, it is usually preferred to place code in other modules. That code can then be invoked from a minimal __main__.py.
    """

    (And then the see-also opens, "See venv for an example of a package with a minimal __main__.py in the standard library." which is a natural extension of the discussion.)

    @taleinat's suggestion works as well (crossed streams).

    @riqts
    Copy link
    Mannequin

    riqts mannequin commented Jan 8, 2022

    Have updated the PR to be in line with the issues leading ideas, specifically Tal's suggestion.

    Thanks

    @ferdnyc
    Copy link
    Mannequin Author

    ferdnyc mannequin commented Jan 13, 2022

    Readding Tal to the nosy list, since my previous comment was inadvertently accompanied by an eviction! (Sorry about that.)

    @AlexWaygood AlexWaygood added the type-bug An unexpected behavior, bug, or error label Jan 13, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ofey404
    Copy link
    Contributor

    ofey404 commented Aug 26, 2022

    @ferdnyc Excuse me, but are you still following up this issue and PR? I tried to review your PR, and post some comments.

    @merwok merwok linked a pull request Aug 26, 2022 that will close this issue
    @ferdnyc
    Copy link
    Contributor

    ferdnyc commented Aug 27, 2022

    @ofey404 It's not my PR, that belongs to @riqts. I just opened the issue.

    @tavallaie
    Copy link
    Contributor

    can anyone check my PR?

    brettcannon pushed a commit that referenced this issue Jul 4, 2024
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 4, 2024
    (cherry picked from commit cb688ba)
    
    Co-authored-by: Ali Tavallaie <[email protected]>
    Co-authored-by: Éric <[email protected]>
    Co-authored-by: Frank Dana <[email protected]>
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 4, 2024
    (cherry picked from commit cb688ba)
    
    Co-authored-by: Ali Tavallaie <[email protected]>
    Co-authored-by: Éric <[email protected]>
    Co-authored-by: Frank Dana <[email protected]>
    brettcannon pushed a commit that referenced this issue Jul 4, 2024
    …H-121386)
    
    gh-90437: Fix __main__.py documentation wording (GH-116309)
    (cherry picked from commit cb688ba)
    
    Co-authored-by: Ali Tavallaie <[email protected]>
    Co-authored-by: Éric <[email protected]>
    Co-authored-by: Frank Dana <[email protected]>
    brettcannon pushed a commit that referenced this issue Jul 4, 2024
    …121385)
    
    gh-90437: Fix __main__.py documentation wording (GH-116309)
    (cherry picked from commit cb688ba)
    
    Co-authored-by: Ali Tavallaie <[email protected]>
    Co-authored-by: Éric <[email protected]>
    Co-authored-by: Frank Dana <[email protected]>
    noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
    estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes docs Documentation in the Doc dir easy type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    8 participants