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

Use pypyoject toml #56

Merged
merged 14 commits into from
Jul 2, 2023
Merged

Use pypyoject toml #56

merged 14 commits into from
Jul 2, 2023

Conversation

frank113
Copy link
Contributor

@frank113 frank113 commented Jun 25, 2023

Summary

This PR responds to issue #49 and migrates setup.py configurations to pyproject.toml.

Issues Addressed

Changes Made

To modernize the structure of this package I largely followed the example by PyPa's sampleproject repository.

  1. Use build to build the package within Makefile.
  2. Remove setup.py and migrate settings to pyproject.toml
  3. Move version hyper parameter from argparse_dataclass.py to the project.version field of pyproject.toml
  4. Add build to dependencies within requirements_dev.txt and pyproject.toml
  5. Add the [project.optional-dependencies] field within pyproject.toml. This inclusion enables the following behavior:
pip install .[dev]
  1. Add readme_renderer to tox.ini as per sampleproject's tox.ini .
  2. Bump version from 2.0.1 to 2.1
  3. Update CHANGELOG.md to track changes

Considerations

  • Documentation references the use of twine to validate the distributions prior to publishing. We can add the following script to build and within the workflow:
twine check dist/*

This change must be accompanied with the addition of twine in the dev requirements. See python documentation and sampleproject's tox.ini .

  • readme_renderer seems to be insurance for issues that may arise in deployment.
  • The version update from 2.0.0 -> 2.0.1 -> 2.1.0 presents a potential headache as 2.0.1 is still unreleased. We can combine the unreleased changes from 2.0.1 into 2.1.0 -- what do you think?

Sources

@frank113 frank113 marked this pull request as ready for review June 25, 2023 22:36
Copy link
Owner

@mivade mivade left a comment

Choose a reason for hiding this comment

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

Thank you for doing this! I've left one minor comment and a question for clarification.

argparse_dataclass.py Show resolved Hide resolved
tox.ini Outdated
envlist = py38, py39, py310, py311
# isolated_build = True

[testenv]
deps =
-r {toxinidir}/requirements_dev.txt
readme_renderer
Copy link
Owner

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for calling this out. This package is intended to check if the README.rst can be rendered on PyPi. The inclusion of this package was inspired by sampleproject's use in tox.ini.

Given the current state of the package's PyPi page I removed it from the dependency list. With that being said we can add it later if we decide to enhance our CI processes or use automated publishing.

Copy link
Owner

Choose a reason for hiding this comment

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

Interesting, thanks.

@mivade mivade merged commit 37baa76 into mivade:main Jul 2, 2023
4 checks passed
@mivade mivade mentioned this pull request Jul 2, 2023
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