Skip to content

do not create root-level "tests" package #213

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

Closed
joelb123 opened this issue Sep 25, 2019 · 3 comments · Fixed by #214
Closed

do not create root-level "tests" package #213

joelb123 opened this issue Sep 25, 2019 · 3 comments · Fixed by #214

Comments

@joelb123
Copy link

Merge pull request #211 moved the tests/ directory from arrayfire.tests to the root, resulting in installation of a root-level python package called "tests". While pip will happily make this in your virtualenv, every sane package manager will complain. Most python coders consider it surprising and therefore bad design for "pip install mypackage" to create another top-level package other than "mypackage". It's even worse to make a top-level package with a common name like "tests", which just asks for conflicts. Please back this commit out.

@roaffix
Copy link
Contributor

roaffix commented Sep 26, 2019

Hello. tests/ dir creation is a bug, because I forgot to add include option in metatag [options.packages.find] in setup.cfg. I'll add another PR to fix this bug ASAP.
And as for the tests in the root, this is a bad practice to keep tests/ within your lib. You will install them every time with your package, but you don't need them. It's okay to keep tests for OS integration or something, but these tests are just simple tests of base functionality.
Also, I didn't get what do you mean by sane package manager 🤔

@roaffix
Copy link
Contributor

roaffix commented Sep 26, 2019

Try to run python setup.py build in PR #214 branch. tests/ dir should not be created now.

@9prady9
Copy link
Member

9prady9 commented Sep 26, 2019

@joelb123 Looks like @roaffix fixed this behavior in #214 . Can you please confirm this isn't happening per your test.

Interesting that current master has tests in exclude list of setup.cfg and yet this happened.

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 a pull request may close this issue.

3 participants