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

chg: dev: add main module. #2453

Merged
merged 5 commits into from
Feb 25, 2017
Merged

chg: dev: add main module. #2453

merged 5 commits into from
Feb 25, 2017

Conversation

rachmadaniHaryono
Copy link
Contributor

@rachmadaniHaryono rachmadaniHaryono commented Feb 24, 2017

add main file for module. with this program can be run from module. e.g. python -m beets. but this require fix on SubcommandOption, because it will print __main__.py instead of program name. so i add another method to inherit parent class and add exception for that word.

i don't know how to test this with unittest, but if threre is introduction for testing this program, that would be helpful and i will add another commit for test.

python version: Python 3.5.2+

Distributor ID: Ubuntu
Description: Ubuntu 16.10
Release: 16.10
Codename: yakkety

@sampsyo
Copy link
Member

sampsyo commented Feb 24, 2017

Hi! Thanks for the contribution. This change seems fine, but can you perhaps explain a little more about why you want it? What situations is this useful for?

Also, please take a look at the style checker. Please add the standard license header and future imports to the new file.

Finally, I'm not thrilled about hard-coding the name "beets" in there. Maybe we can just leave that part unmodified? It seems unlikely to bother anyone that the program name will look a little weird...

@rachmadaniHaryono
Copy link
Contributor Author

Hi! Thanks for the contribution. This change seems fine, but can you perhaps explain a little more about why you want it? What situations is this useful for?

before i explain here is this related SO question http://stackoverflow.com/questions/4042905/what-is-main-py, and docs from python https://docs.python.org/3/library/__main__.html

for myself, i have to use several python version and keep alias to those python. so if i have to run on python3, where the default is python2.7, i have to add python -m package rather than using the command line given by the package.

Finally, I'm not thrilled about hard-coding the name "beets" in there. Maybe we can just leave that part unmodified? It seems unlikely to bother anyone that the program name will look a little weird...

i can't think any other better way. after install the package for testing on virtual env (pip install -e .), the prog variable will always be replaced with basename of the file it run (in this case __main__.py on python -m beets --help). other way i can think is take directory name of __main__.py (beets) but i don't think any advantage of it than hardcoded value.

note: and i just realize that on setup.py the entry point of the program is beet without s. the folder project is beets, the github is also beets.

this is maybe one of the error when i try on those different python environment.

@sampsyo
Copy link
Member

sampsyo commented Feb 24, 2017

I see—you'd like to distinguish between python3 -m beets and python2 -m beets. Is that right? Because, on my machine at least, if I pip2 install beets or pip3 install beets, then I get a beet executable that refers to the correct version of Python—independent of which python is on my path. Have you tried that?

About the executable name: let's just leave it as it is. I actually think using __main__.py in the output is actually more correct when you invoke beets that way—you haven't run beet; you've invoked __main__.py. See, for example, the output of python3 -m http.server --help.

@rachmadaniHaryono
Copy link
Contributor Author

I see—you'd like to distinguish between python3 -m beets and python2 -m beets. Is that right?

then I get a beet executable that refers to the correct version of Python—independent of which python is on my path. Have you tried that?

i have tried, but due to confusion between python, virtual env and beets vs beet this doesnt make it clear. using only pip don't clearly tell on which python it will be installed (although using pip2 and pip3 make it more helpful.

See, for example, the output of python3 -m http.server --help

the help actually print server.py instead of __main__.py

but i will leave leave the prog value up to python in new commit.

@sampsyo
Copy link
Member

sampsyo commented Feb 25, 2017

OK, thanks for explaining.

As I mentioned above, can you please add the standard license comment and future imports to the top of the new file?

Also, the docstring "main module" is not terribly helpful by itself. Could you please write a full sentence or two about the purpose there, or else just delete it?

Finally, we'll need a changelog entry. Thanks!

@rachmadaniHaryono
Copy link
Contributor Author

add license comments and future import. is the changelog entry corect?

@sampsyo
Copy link
Member

sampsyo commented Feb 25, 2017

Thank you! Merged. ✨

@sampsyo sampsyo merged commit 0fe2279 into beetbox:master Feb 25, 2017
sampsyo added a commit that referenced this pull request Feb 25, 2017
sampsyo added a commit that referenced this pull request Feb 25, 2017
sampsyo added a commit that referenced this pull request Feb 25, 2017
@rachmadaniHaryono rachmadaniHaryono deleted the feature/add-main branch February 25, 2017 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants