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

find_py_modules? #1580

Open
anntzer opened this issue Nov 7, 2018 · 5 comments
Open

find_py_modules? #1580

anntzer opened this issue Nov 7, 2018 · 5 comments
Labels
Needs Discussion Issues where the implementation still needs to be discussed.

Comments

@anntzer
Copy link

anntzer commented Nov 7, 2018

AFAICT, there is no find_py_modules() function that could be used as value to setup(py_modules=...), similarly to how one can use setup(packages=find_packages()).

Such a function would be a useful addition, providing benefits similar to find_packages().

@pganssle
Copy link
Member

pganssle commented Nov 7, 2018

I don't think it makes sense to do that. I personally find find_packages very dangerous, but it's necessary because you need to specify all modules and submodules for your package. The fact that you can have modules nested under other modules is why it's necessary to have a function that recursively finds modules.

py_modules is for python files that are themselves modules. In general, the best practice would be to have exactly one of these, and it's very likely that you have other .py files in your root directory that are not intended to be used as modules, so find_py_modules would be even more dangerous than find_packages but with none of the benefits.

That said, do you have a specific use case in mind?

@anntzer
Copy link
Author

anntzer commented Nov 7, 2018

It's true that I am basically always using a src-type layout these days so I didn't think of the issue of find_py_modules finding e.g. setup.py itself in a non-src-type layout. OTOH we can easily make it raise if where == ""...

I don't think either find_packages would be significantly harder to write than find_py_modules (at least nowadays), so the complexity argument doesn't really hold: up to exclude and include (which would take the same amount of code for filtering in either), they're basically (untested, but you get the idea):

def find_packages(where): 
    return [*{*(str(path.relative_to(where).parent)
                for path in Path().glob(where + "/**/__init__.py"))}]

def find_py_modules(where): 
    return [*{*(str(path.relative_to(where).parent)
                for path in Path().glob(where + "/*.py"))}]

The use case is to write generic setup.py's that are reusable (in the style of cookiecutter, but without using a templating engine).

@pganssle
Copy link
Member

pganssle commented Nov 7, 2018

The implementation is not the problem, the fact that the implementation is easy is actually one reason not to do it, since anyone who wants to do this fairly unusual thing can implement it themselves and make it specific enough to their workflow to avoid some of the edge cases you are likely to get in a general implementation.

More importantly, py_modules is mostly (only?) useful for when you have no top level package. In all other situations find_packages and find_namespace_packages will suffice. So py_modules is really only useful when it's a list of one file, or if you are deploying multiple packages from one setup.py, which is a really bad idea. find_py_modules would only really be useful in that last case and I don't want to encourage or facilitate such a thing.

@anntzer
Copy link
Author

anntzer commented Nov 7, 2018

I only raised the issue of complexity because you wrote

it's necessary because you need to specify all modules and submodules for your package.

which I interpreted as implying that it was too hard to write otherwise(?)

Anyways, feel free to close if this is not going to happen.

@pganssle
Copy link
Member

pganssle commented Nov 7, 2018

Ah, no, I meant the opposite. py_modules is for when your source tree is basically a single Python file. There are no submodules, and if you're doing things right, it's a 1-element list. In such a simple module, you don't need a function to find all the files that you want to package up, because the list is and always will be one item (once you get to 2 items, you need to refactor into a module with an __init__.py, because now you have to have submodules). It's not that it's too hard to write (though making a perfectly general version of this that does the right thing might be hard), it's that it's unnecessary because this is not something that needs to be done programmatically.

In my opinion, ideally you also wouldn't need find_packages, since most people just want to use their folder structure as the module structure unchanged (like it is if you import from the module when your CWD is at the base of the repo and you aren't using the src layout), but that has its own problems (for example many modules put a tests submodule that isn't shipped with the package, which is also an anti-pattern in my opinion), and in any case it would be a major backwards-incompatible change.

We can leave the ticket open for a bit so other people can weigh in, but I'm definitely -1 on this idea. The only realistic use case would be an anti-pattern, and it has the potential to cause a lot of problems if used incorrectly (and most people will just guess how it's supposed to be used based on the name, not look up the documentation on it).

@pganssle pganssle added the Needs Discussion Issues where the implementation still needs to be discussed. label Nov 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Issues where the implementation still needs to be discussed.
Projects
None yet
Development

No branches or pull requests

2 participants