-
Notifications
You must be signed in to change notification settings - Fork 2k
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
refactor: update package metadata #3079
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a ton Ofek, this is great!
Ok for me to remove the capping for Python
COVERAGE_FILE = test-reports/.coverage | ||
PYTEST_ADDOPTS = --junitxml=test-reports/{envname}/junit.xml -vv | ||
commands = | ||
coverage run --source haystack --parallel-mode -m pytest {posargs} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for someone else from @deepset-ai/core-engineering to also review before merging but I'm super happy about this change, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks a ton for this change!! 🎉
I found one small change to do in the list of files and folders we package, but other than that it's ready to go for me. Thanks again!
pyproject.toml
Outdated
include = [ | ||
"/haystack", | ||
"/rest_api", | ||
"/VERSION.txt", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to ship the REST API with the package (probably now they're shipped even if they shouldn't). On the other hand, we should rather include LICENSE
?
include = [ | |
"/haystack", | |
"/rest_api", | |
"/VERSION.txt", | |
] | |
include = [ | |
"/haystack", | |
"/LICENSE", | |
"/VERSION.txt", | |
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Licenses are always included https://hatch.pypa.io/latest/plugins/builder/sdist/#default-file-selection
Removed! Yes I was following sdist on PyPI:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you once more! 😊
Happy to help! Cool project btw 🥇 |
Background
Hello there! The Python packaging ecosystem has standardized on the interface for build backends (PEP 517/PEP 660) and the format for metadata declaration (PEP 621/PEP 631). As a result, the execution of
setup.py
files is now deprecated.So, I'm spending my free time updating important projects so that they are modernized and set an example for others 😄
Proposed Changes
This implements PEP 621, obviating the need for
setup.py
andsetup.cfg
. The build backendhatchling
(of which I am a maintainer in the PyPA) is now used as that is the default in the official Python packaging tutorial. Hatchling is available on all the major distribution channels such as Debian, Fedora, Arch Linux, conda-forge, Nixpkgs, Alpine Linux, FreeBSD/OpenBSD, Gentoo Linux, MacPorts, OpenEmbedded, Spack, etc.How did you test it?
Notes for the reviewer
*.egg-info
frompython setup.py develop
; this is now fixedtox
againChecklist