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

Packaging #4

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Packaging #4

wants to merge 3 commits into from

Conversation

exhuma
Copy link
Contributor

@exhuma exhuma commented Jun 23, 2021

This PR covers issue #1

There are a few changes made here:

  • The README is updated with the new installation procedure:
    • It includes a command to install directly from pypi which requires the package to be published of course. It may need to be updated if the pypi package is named differently.
    • It mentions virtual-environments which are good to keep the system clean.
    • The "execution" command is updated to reflect the console entry-point provided with this updated version
  • The version is set to 2.0 because it's backwards compatible. It may make sense to use calendar-versioning instead? @srijan what do you prefer?
  • Dependencies are specified without version boundaries. This is good for development, but on release-time it would be good to freeze the versions in a requirements.txt file. For now, naïve dependencies should be more than sufficient.
  • The TROVE classifiers are a bit handwavy. I've included the link to valid pypi classifiers should we need to modify them.
  • Because the TOML dependency is now provided by the setup.py definnition (and virtual-env), the import-fallback has become redundant and has been removed.
  • The set_cwd() is problematic with packaging. It's better to let the user specify the path to the file manually. It causes less confusion. It's better to see the installaed package as an immutable "frozen" box and keep files like the config-file and query-files outside of that box. I would suggest using the argparse to make usage a bit easier.

Thoughts?

@exhuma
Copy link
Contributor Author

exhuma commented Jul 8, 2021

I have just learned today that setuptools now also supports PEP-517 (https://setuptools.readthedocs.io/en/latest/build_meta.html)

I will change the files accordingly. To be a bit more "modern" 😉

@exhuma
Copy link
Contributor Author

exhuma commented Jul 8, 2021

Done. It's now PEP-517 compliant.

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.

1 participant