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

tests directory is installed with pip install #588

Closed
3 tasks done
nalepae opened this issue Mar 3, 2021 · 6 comments · Fixed by #589
Closed
3 tasks done

tests directory is installed with pip install #588

nalepae opened this issue Mar 3, 2021 · 6 comments · Fixed by #589

Comments

@nalepae
Copy link

nalepae commented Mar 3, 2021

Problem description

Version: 4.2.0

Observed:

$ pip install smart-open --target /tmp/smart-open
$ cd /tmp/smart-open/smart_open
$ du -hcs *

We get:

16K	azure.py
8,0K	bytebuffer.py
4,0K	compression.py
4,0K	concurrency.py
4,0K	constants.py
8,0K	doctools.py
20K	gcs.py
4,0K	hdfs.py
12K	http.py
4,0K	__init__.py
4,0K	local_file.py
160K	__pycache__
44K	s3.py
16K	smart_open_lib.py
8,0K	ssh.py
524K	tests
4,0K	transport.py
8,0K	utils.py
4,0K	version.py
8,0K	webhdfs.py
864K	total

==> Tests files represent 60% of the total package size.
These files are not needed at "run time".

Expected:
==> Tests files are not installed with $ pip install. (So the size of the package goes down to 340 kB.)

Additional information:
If this issue is validated, I can fix it and provide a PR.
The size of packages are important when using AWS Lambdas. (We want to keep Lambdas size as small as possible.)

Checklist

Before you create the issue, please make sure you have:

  • Described the problem clearly
  • Provided a minimal reproducible example, including any required data
  • Provided the version numbers of the relevant software
@petedannemann
Copy link
Contributor

I think we can exclude this with the following in setup.py

packages=find_packages(exclude=['smart_open.test'])

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 4, 2021

@piskvorky Are we including tests in the package intentionally? If not, then perhaps it would be a good idea to remove them.

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 4, 2021

I skimmed through the commit logs and found this:

52d300f (#401)

So it looks like the tests and their data are packaged intentionally.
@jayvdb Do you remember what the motivation behind this was?
I think now is a decent time to review that decision and see if it's still worthwhile.

@piskvorky
Copy link
Owner

piskvorky commented Mar 4, 2021

That PR included data required by smart_open tests, so that the tests pass.

I think what's being asked here is different: remove the tests from the packaged distribution altogether, both code and data.

FWIW I have no objection, the packaged tests serve no purpose to me. Personally when I want to test a package, I go full dev-mode and install from git and tinker (as opposed to test a pip install-ed prepackaged distribution). But other people may have different valid use-cases.

@nalepae
Copy link
Author

nalepae commented Mar 4, 2021

So I guess I can open a PR?

@e-nalepa
Copy link
Contributor

e-nalepa commented Mar 4, 2021

I just opened #589.

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.

5 participants