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

Module with __init__.pyx is not recognized by setuptools.find_packages() #1509

Open
ghost opened this issue Oct 13, 2018 · 6 comments
Open
Labels
enhancement Needs Investigation Issues which are likely in scope but need investigation to figure out the cause

Comments

@ghost
Copy link

ghost commented Oct 13, 2018

A module with a Cythonized init.pyx (but other files being a *.py file) is not recognized by setuptools.find_packages():

$ mkdir mymodule
$ touch mymodule/__init__.pyx mymodule/__init__.cpython-36m-x86_64-linux-gnu.so mymodule/__init__.c
$ python3 -c "import setuptools; print(setuptools.find_packages('.'))"
[]
$ touch mymodule/__init__.py
$ python3 -c "import setuptools; print(setuptools.find_packages('.'))"
['mymodule']

However, if I put an __init__.py in there as well, I don't think I can properly import the Cython .so module anymore. Is this a bug of sorts? I don't know what to do about this...

@ghost ghost changed the title Module with __init__.pyx is not recognize by setuptools.find_packages() Module with __init__.pyx is not recognized by setuptools.find_packages() Oct 13, 2018
@pganssle pganssle added Needs Triage Issues that need to be evaluated for severity and status. Needs Investigation Issues which are likely in scope but need investigation to figure out the cause enhancement and removed Needs Triage Issues that need to be evaluated for severity and status. labels Oct 19, 2018
@pganssle
Copy link
Member

I doubt it would hurt much to have find_packages detect packages that have an __init__.pyx instead of an __init__.py, but I find find_packages very dangerous to change (and often, to use), because it makes it super easy to accidentally add a bunch of erroneous packages.

@Jonast For now, I would just manually specify the package you want like this:

packages = setuptools.find_packages('.') + ['mypkg.mod_with_initpyx']

Also, on the subject of "find_packages is dangerous", here is a very compelling case for using the src layout, which obviates a lot of the danger factor.

@ghost
Copy link
Author

ghost commented Oct 23, 2018

I already use an src folder, the example above is just a minimal reproduction case I isolated to find out what on earth is going on with my broken setup.py 😄 and I got a similar workaround in place, but it is ugly.

Then again, Cython itself has a bug that breaks __init__.pyx packages on windows: cython/cython#2665 - so right now I'm forced to use an ugly import redirection anyway. So it's not like this issue is super high on my priority list right now.

But it'd be nice to see it solved properly in setuptools, eventually.

@ghost
Copy link
Author

ghost commented Jan 11, 2019

For what it's worth, Cython has now fixed this bug, so the only remaining issue with this sort of module is this setuptools bug here.

Is there chance this limitation will be fixed? It's a bit ugly that find_packages doesn't pick this up properly

@pganssle
Copy link
Member

@Jonast I have no opposition to fixing it in principle, I'm just mildly worried that it might cause unforeseen problems. Feel free to send a PR. I'm not sure what kind of research we can do that would give us confidence in merging and releasing something that changes the behavior of find_packages.

One possibility (and I'm not sure how the other maintainers feel about this), is that we can add a "feature flag" here to ease in this change, so the change would roll out in 3 stages:

  1. Add **kwargs to find_packages and check if cything_init_support is passed (the only valid value for this would be True).. Calculation takes place in two stages: first one includes __init__.pyx packages, but they are marked as such. If cython_init_support is not specified and some __init__.pyx modules are filtered out, raise a warning that says something like, "You have packages that are not included because they only have an __init__.pyx, in the future these will be included by default. Either set cython_init_support=True or explicitly filter out these packages."
  2. Change the default behavior of find_packages to include __init__.pyx packages and detect if cython_init_support is passed. Start warning that that the cython_init_support toggle will be removed in the future, and that you should explicitly filter out any packages that need to be removed.
  3. Start raising an error if cython_init_support is passed.

A stronger version of step 1 would be to actually raise an error if any __init__.pyx modules would be filtered out, thus forcing people to either toggle cython support or filter out the relevant modules explicitly.

This whole thing may be a bit overboard, though. I'd be willing to be convinced that it's not such a big deal to add this without the complicated rollout.

@pganssle
Copy link
Member

Actually, one other thing to consider here - maybe we should be somewhat getting out of the business of explicitly supporting Cython and instead try to delegate that to a Cython-specific plugin (or Cython itself)?

We could instead provide some hooks in find_namespace and find_namespace_packages for downstream libraries to add their own glob patterns, and users can specify the relevant plugins using PEP 518 build requirements. Maybe it's only useful for Cython, in which case what I'm suggesting is far too general, but I wonder if maybe people building backends with other languages or frameworks would find it useful.

@ghost
Copy link
Author

ghost commented Jan 12, 2019

I assume this is at its core mostly a one-line change and the difficulty lies in the rollout. Therefore I suggest someone who has done such a rollout and has a better idea on how a deprecation mechanism should be put in place would be better suited for taking a look at this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Needs Investigation Issues which are likely in scope but need investigation to figure out the cause
Projects
None yet
Development

No branches or pull requests

1 participant