Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

moved main() into __main__.py to keep __init__.py minimal #67

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

Open
seancmonahan wants to merge 2 commits into pypa:main
base: main
Choose a base branch
Loading
from seancmonahan:master

Conversation

@seancmonahan
Copy link

@seancmonahan seancmonahan commented Mar 29, 2018

print("Call your main application code here")


if __name__ == "__main__":
Copy link
Contributor

Choose a reason for hiding this comment

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

__name__ will never not equal __main__ in __main__.py.

Copy link
Author

@seancmonahan seancmonahan Apr 9, 2018

Choose a reason for hiding this comment

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

__name__ equals sample.__main__ once the project is installed and run via the executable 'sample' generated by setup.py.

Without the if __name__ == "__main__": check (but still calling main() at the end of __main__.py):

$ virtualenv foo
$ source foo/bin/activate
(foo) $ pip install ../path/to/sampleproject
(foo) $ sample #i.e. foo/bin/sample
Call your main application code here
Call your main application code here
(foo) $

Copy link
Author

@seancmonahan seancmonahan Apr 9, 2018

Choose a reason for hiding this comment

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

I suppose the same effect could be had by reverting __init__.py and setup.py, and making __main__.py be (like pip's __main__.py):

from . import main
if __name__ == '__main__':
 main()

My motivation was twofold though:

  • Make the package runnable via python -m sample
  • Keep __init__.py as clean as possible.

For example, pip's primary __init__.py is a single line definition of __version__, and setup.py uses pip._internal:main as its console_script entrypoint. (I do see that ultimately main() is defined inside pip/_internal/__init__.py ̄\_(ツ)_/ ̄ )

Regardless of where def main(): ends up, my first goal (to make the package runnable) requires the addition of a __main__.py.

Copy link
Contributor

@dmtucker dmtucker Apr 9, 2018

Choose a reason for hiding this comment

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

Ah, you're right about __name__ during import...

IMO, the double execution is evidence of abuse. __main__.py is meant to control the behavior of python -m, yet here it is tangled up in the execution of the entry point.
Instead of __main__.py and the entry point leading the user to the same call (e.g. sample.cli.main()), the latter leads to the former which has to have a special hack added to make things work.

The docs even describe __main__.py as the equivalent of if __name__ == '__main__' for packages (implying packages should not need it):

For a package, the same effect can be achieved by including a __main__.py module, the contents of which will be executed when the module is run with -m.

Copy link
Author

@seancmonahan seancmonahan Apr 10, 2018

Choose a reason for hiding this comment

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

Oops, you're correct about the double-execution behavior. Seems that I was accidentally running from a virtualenv with an outdated install of sample with the console_script entrypoint pointing to __main__ still! Should I close this pull request?

Would a single patch for adding a __main__.py consisting solely of

from . import main
main()

be accepted/useful? It would allow the package to be run via python -m sample and avoid the nonsense I would've introduced by moving def main(): into __main__.py.

Also, would adding a sample/cli.py module be in the scope of this project? It seems like a useful way to make minimize __init__.py to either an empty file or a single from .cli import main. (This does trigger a flake8 error for an unused import.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that is a reasonable __main__.py, and I would say a cli.py module is in-scope for the project since someone previously defined an entry_point.

Not sure how we want to manage imports in __init__.py... I think generally people use that file to define the package API (which often implies unused imports), but it's not explicitly required since modules within the package can be imported independently. I'd probably just leave it empty to avoid a # noqa.

Copy link
Member

Choose a reason for hiding this comment

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

I still think this is not something that has an "obvious" best practice approach (as evidenced by the discussion going on here) and users would be better served with documentation that discussed the various approaches and the pros and cons, rather than having one particular approach added to this sample project. Let's also not forget that most projects don't even need to be runnable.

A new section in https://packaging.python.org/guides/ would likely be a better way of dealing with this, IMO.

Base automatically changed from master to main January 21, 2021 18:40
Copy link
Member

Hi @seancmonahan. Are you still interested on this PR? If you are, I recommend you to edit the first post of the PR, and add the following expression: Closes #141. That way, that issue (#141, which is directly related) will be closed if this gets merged. Also, can you rebase the PR?

Copy link
Member

Hi everyone! Are we still interested in this PR?

Copy link
Member

pfmoore commented Dec 30, 2022

My objection to doing this, which I noted above, still stands.

DiddiLeija reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pfmoore pfmoore pfmoore left review comments

+1 more reviewer

@dmtucker dmtucker dmtucker left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /