Include library folder in Python packages
#184
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…age through MANIFEST and allow their installation with setuptools
…ly to the Docker image
|
Hey and thank you soo much for opening this PR. I think we could leave the |
elad-pticha
reviewed
Jun 16, 2024
MANIFEST.in
Outdated
| @@ -1,3 +1,4 @@ | |||
| include README.md LICENSE requirements.txt main.py | |||
| recursive-include src/library *.yml | |||
There was a problem hiding this comment.
Change to library *.yml
There was a problem hiding this comment.
This is the whole point of this PR, right now you are not including the library within the Python package, and the culprit is that the library it outside the package source, you could maybe do a build hook with setuptools but that is way more complicated than just moving it into the source tree.
See below the build log, the yml files are never included:
$ python setup.py bdist_wheel
running bdist_wheel
running build
running build_py
creating build
creating build/lib
creating build/lib/src
copying src/__init__.py -> build/lib/src
copying src/cmdline.py -> build/lib/src
creating build/lib/src/common
copying src/common/__init__.py -> build/lib/src/common
copying src/common/ignore_warnings.py -> build/lib/src/common
copying src/common/utils.py -> build/lib/src/common
creating build/lib/src/config
copying src/config/__init__.py -> build/lib/src/config
copying src/config/config.py -> build/lib/src/config
creating build/lib/src/downloader
copying src/downloader/__init__.py -> build/lib/src/downloader
copying src/downloader/download.py -> build/lib/src/downloader
copying src/downloader/gh_api.py -> build/lib/src/downloader
copying src/downloader/utils.py -> build/lib/src/downloader
creating build/lib/src/indexer
copying src/indexer/__init__.py -> build/lib/src/indexer
copying src/indexer/index.py -> build/lib/src/indexer
creating build/lib/src/logger
copying src/logger/__init__.py -> build/lib/src/logger
copying src/logger/log.py -> build/lib/src/logger
creating build/lib/src/queries
copying src/queries/__init__.py -> build/lib/src/queries
creating build/lib/src/reporter
copying src/reporter/__init__.py -> build/lib/src/reporter
copying src/reporter/report.py -> build/lib/src/reporter
copying src/reporter/slack_reporter.py -> build/lib/src/reporter
creating build/lib/src/storage
copying src/storage/__init__.py -> build/lib/src/storage
copying src/storage/neo4j_graph.py -> build/lib/src/storage
copying src/storage/neo4j_utils.py -> build/lib/src/storage
copying src/storage/redis_connection.py -> build/lib/src/storage
copying src/storage/redis_utils.py -> build/lib/src/storage
creating build/lib/src/workflow_components
copying src/workflow_components/__init__.py -> build/lib/src/workflow_components
copying src/workflow_components/composite_action.py -> build/lib/src/workflow_components
copying src/workflow_components/dependency.py -> build/lib/src/workflow_components
copying src/workflow_components/parsing_utils.py -> build/lib/src/workflow_components
copying src/workflow_components/workflow.py -> build/lib/src/workflow_components
/usr/lib/python3.12/site-packages/setuptools/_distutils/cmd.py:66: SetuptoolsDeprecationWarning: setup.py install is deprecated.
!!
********************************************************************************
Please avoid running ``setup.py`` directly.
Instead, use pypa/build, pypa/installer or other
standards-based tools.
See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
********************************************************************************
!!
self.initialize_options()
installing to build/bdist.linux-x86_64/wheel
running install
running install_lib
creating build/bdist.linux-x86_64
creating build/bdist.linux-x86_64/wheel
creating build/bdist.linux-x86_64/wheel/src
copying build/lib/src/__init__.py -> build/bdist.linux-x86_64/wheel/src
copying build/lib/src/cmdline.py -> build/bdist.linux-x86_64/wheel/src
creating build/bdist.linux-x86_64/wheel/src/common
copying build/lib/src/common/__init__.py -> build/bdist.linux-x86_64/wheel/src/common
copying build/lib/src/common/ignore_warnings.py -> build/bdist.linux-x86_64/wheel/src/common
copying build/lib/src/common/utils.py -> build/bdist.linux-x86_64/wheel/src/common
creating build/bdist.linux-x86_64/wheel/src/config
copying build/lib/src/config/__init__.py -> build/bdist.linux-x86_64/wheel/src/config
copying build/lib/src/config/config.py -> build/bdist.linux-x86_64/wheel/src/config
creating build/bdist.linux-x86_64/wheel/src/downloader
copying build/lib/src/downloader/__init__.py -> build/bdist.linux-x86_64/wheel/src/downloader
copying build/lib/src/downloader/download.py -> build/bdist.linux-x86_64/wheel/src/downloader
copying build/lib/src/downloader/gh_api.py -> build/bdist.linux-x86_64/wheel/src/downloader
copying build/lib/src/downloader/utils.py -> build/bdist.linux-x86_64/wheel/src/downloader
creating build/bdist.linux-x86_64/wheel/src/indexer
copying build/lib/src/indexer/__init__.py -> build/bdist.linux-x86_64/wheel/src/indexer
copying build/lib/src/indexer/index.py -> build/bdist.linux-x86_64/wheel/src/indexer
creating build/bdist.linux-x86_64/wheel/src/logger
copying build/lib/src/logger/__init__.py -> build/bdist.linux-x86_64/wheel/src/logger
copying build/lib/src/logger/log.py -> build/bdist.linux-x86_64/wheel/src/logger
creating build/bdist.linux-x86_64/wheel/src/queries
copying build/lib/src/queries/__init__.py -> build/bdist.linux-x86_64/wheel/src/queries
creating build/bdist.linux-x86_64/wheel/src/reporter
copying build/lib/src/reporter/__init__.py -> build/bdist.linux-x86_64/wheel/src/reporter
copying build/lib/src/reporter/report.py -> build/bdist.linux-x86_64/wheel/src/reporter
copying build/lib/src/reporter/slack_reporter.py -> build/bdist.linux-x86_64/wheel/src/reporter
creating build/bdist.linux-x86_64/wheel/src/storage
copying build/lib/src/storage/__init__.py -> build/bdist.linux-x86_64/wheel/src/storage
copying build/lib/src/storage/neo4j_graph.py -> build/bdist.linux-x86_64/wheel/src/storage
copying build/lib/src/storage/neo4j_utils.py -> build/bdist.linux-x86_64/wheel/src/storage
copying build/lib/src/storage/redis_connection.py -> build/bdist.linux-x86_64/wheel/src/storage
copying build/lib/src/storage/redis_utils.py -> build/bdist.linux-x86_64/wheel/src/storage
creating build/bdist.linux-x86_64/wheel/src/workflow_components
copying build/lib/src/workflow_components/__init__.py -> build/bdist.linux-x86_64/wheel/src/workflow_components
copying build/lib/src/workflow_components/composite_action.py -> build/bdist.linux-x86_64/wheel/src/workflow_components
copying build/lib/src/workflow_components/dependency.py -> build/bdist.linux-x86_64/wheel/src/workflow_components
copying build/lib/src/workflow_components/parsing_utils.py -> build/bdist.linux-x86_64/wheel/src/workflow_components
copying build/lib/src/workflow_components/workflow.py -> build/bdist.linux-x86_64/wheel/src/workflow_components
running install_egg_info
running egg_info
creating raven_cycode.egg-info
writing raven_cycode.egg-info/PKG-INFO
writing dependency_links to raven_cycode.egg-info/dependency_links.txt
writing entry points to raven_cycode.egg-info/entry_points.txt
writing requirements to raven_cycode.egg-info/requires.txt
writing top-level names to raven_cycode.egg-info/top_level.txt
writing manifest file 'raven_cycode.egg-info/SOURCES.txt'
reading manifest file 'raven_cycode.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'LICENSE'
adding license file 'LICENSE.md'
writing manifest file 'raven_cycode.egg-info/SOURCES.txt'
Copying raven_cycode.egg-info to build/bdist.linux-x86_64/wheel/raven_cycode-0.0.0-py3.12.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/raven_cycode-0.0.0.dist-info/WHEEL
creating 'dist/raven_cycode-0.0.0-py3-none-any.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding 'src/__init__.py'
adding 'src/cmdline.py'
adding 'src/common/__init__.py'
adding 'src/common/ignore_warnings.py'
adding 'src/common/utils.py'
adding 'src/config/__init__.py'
adding 'src/config/config.py'
adding 'src/downloader/__init__.py'
adding 'src/downloader/download.py'
adding 'src/downloader/gh_api.py'
adding 'src/downloader/utils.py'
adding 'src/indexer/__init__.py'
adding 'src/indexer/index.py'
adding 'src/logger/__init__.py'
adding 'src/logger/log.py'
adding 'src/queries/__init__.py'
adding 'src/reporter/__init__.py'
adding 'src/reporter/report.py'
adding 'src/reporter/slack_reporter.py'
adding 'src/storage/__init__.py'
adding 'src/storage/neo4j_graph.py'
adding 'src/storage/neo4j_utils.py'
adding 'src/storage/redis_connection.py'
adding 'src/storage/redis_utils.py'
adding 'src/workflow_components/__init__.py'
adding 'src/workflow_components/composite_action.py'
adding 'src/workflow_components/dependency.py'
adding 'src/workflow_components/parsing_utils.py'
adding 'src/workflow_components/workflow.py'
adding 'raven_cycode-0.0.0.dist-info/LICENSE.md'
adding 'raven_cycode-0.0.0.dist-info/METADATA'
adding 'raven_cycode-0.0.0.dist-info/WHEEL'
adding 'raven_cycode-0.0.0.dist-info/entry_points.txt'
adding 'raven_cycode-0.0.0.dist-info/top_level.txt'
adding 'raven_cycode-0.0.0.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
elad-pticha
reviewed
Jun 16, 2024
|
|
||
| # Run RAVEN tests | ||
| CMD ["make", "test-run"] | ||
| CMD ["make", "test-run"] |
There was a problem hiding this comment.
Remove extra line added
elad-pticha
reviewed
Jun 16, 2024
| install_requires=REQUIRMENTS, | ||
| packages=find_packages(exclude=("tests", "tests.*")), | ||
| entry_points={"console_scripts": ["raven = src.cmdline:execute"]}, | ||
| include_package_data=True, |
There was a problem hiding this comment.
Is this required?
I tried to build the package without this, and it worked. Is there something I am missing?
There was a problem hiding this comment.
Just a safeguard, it looks like it's at True by default but the package wants the data to be included, so I'd suggest to keep it explicit.
elad-pticha
suggested changes
Jun 16, 2024
|
Hey man, the package you are distributing is still broken, and the reason is simple, you do not ship the library in the Python package you distribute on pypi.org. Take a look at the following: As you can see, the library is never shipped. In addition, your package is deployed on systems using If you build the source dist: if you build the Now, I can eventually install the package and run the Hope that helps, sorry for the huge delay in helping, I actually just needed to run that again. I believe it's a very helpful tool, I just want everyone to be able to use it ! |
Yggdrasill501
approved these changes
Feb 22, 2025
|
Hey! A few points about the changes: I'm not sure if the Let me know what you think! |
|
@elad-pticha if I understand there is two points where you would like further explanation:
For the first point, I would defer to my last comment who explains why I believe this is necessary. Without it, you are shipping code onto people's computer in a namespace that could conflict with other packages, plus it makes your own Python code do imports like For the second point, what is it that you guys would like to do ? Do you want to ship the library inside the package or not ? Right now, the tool uses a default path value to look for library files and fails (see #183), so I assumed you want that. If there is an update in the library only once a year, you could just a new pypi release with the updated files, but if you update it every other week, then yes I would understand you being on the look for another solution. I've made a lot of assumptions about what you want to achieve, which I could be wrong with, but from a Pythonic standpoint, I maintain my suggestions :) |
|
@BastienFaure Thank you again for taking the time! Regarding the second point, I think it’s a good idea to place the I also agree with your first point, but I find the WDYT? |
|
For having the source code under So for the |
|
@BastienFaure For example, requests follows the And yes, we would like to keep the library folder at the root of the project while still shipping it. |
|
@elad-pticha thank you for the feedback. I'm going to look at alternative to ship the library within the package, I believe there is a prebuild hook in setuptools. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a proposed fix for #183