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

using setuptools.find_packages to wrap modules #141

Merged
merged 2 commits into from Aug 1, 2016
Merged

Conversation

brunobord
Copy link
Member

No description provided.

@@ -1 +1,2 @@
include README.rst
prune workalendar/tests*
Copy link
Contributor

Choose a reason for hiding this comment

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

Strange way do to that. I though adding tests in the package was to ship them as part of the code.
If you do not want to ship them you should probably put them outside the package.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure I could still run the tests effectively, but I may try to remove the test directory from the package, right.

respectfully, I can't see any use-case where you would need to import test packages at runtime. If you "pip install" stuff, you probably just want the part that works. Tests are just meant to be imported / loaded in a test env, but they're probably not a runtime package.

>>> from workalendar.tests.test_europe import FranceTest
>>> # Then what?

Copy link
Contributor

@Natim Natim Jun 30, 2016

Choose a reason for hiding this comment

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

I can't see any use-case where you would need to import test packages at runtime.

When your python package is packaged for Debian, they want to run the tests to see if the package is correctly packages.

$ pip install workalendar nose
$ nosetests workalendar
.......................................................................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 343 tests in 0.070s

OK
$ pip install workalendar py.test
$ py.test --pyargs workalendar
=================================== test session starts ===================================
platform linux2 -- Python 2.7.11+, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /home/demo, inifile: 
collected 343 items 

workalendar/workalendar/tests/test_africa.py .......
workalendar/workalendar/tests/test_america.py .............
workalendar/workalendar/tests/test_asia.py ......
workalendar/workalendar/tests/test_canada.py ....................
workalendar/workalendar/tests/test_core.py .........................
workalendar/workalendar/tests/test_europe.py ....................................................................................................
workalendar/workalendar/tests/test_oceania.py ....................................................................
workalendar/workalendar/tests/test_usa.py ........................................................................................................

=============================== 343 passed in 0.35 seconds ================================

@Natim
Copy link
Contributor

Natim commented Jun 29, 2016

I like the fact that you use find_package I don't like the fact that you exclude tests from the package that way.

@brunobord
Copy link
Member Author

a bit confused by this: https://github.com/novafloss/workalendar/blob/find-packages/setup.py#L10-L13

shall I drop it?

@Natim
Copy link
Contributor

Natim commented Jul 19, 2016

Yes you can drop the distutils one.

@Natim
Copy link
Contributor

Natim commented Jul 19, 2016

It looks good to me 👍

@brunobord brunobord merged commit 975e142 into master Aug 1, 2016
@brunobord brunobord deleted the find-packages branch August 1, 2016 14:38
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.

None yet

2 participants