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.

Sign up for GitHub

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

Jump to bottom

Migrate setup.cfg to pyproject.toml and refactor pyproject.toml #685

Merged
merged 20 commits into from
Mar 29, 2022
Merged

Migrate setup.cfg to pyproject.toml and refactor pyproject.toml #685

merged 20 commits into from
Mar 29, 2022

Conversation

marvinpfoertner
Copy link
Collaborator

In a Nutshell

This PR migrates setuptools configuration from setup.cfg to pyproject.toml and refactors pyproject.toml

Detailed Description

  • migrate setuptools configuration from setup.cfg to pyproject.toml
  • update black configuration
  • update isort configuration
  • remove requirements.txt

Related Issues

None

@marvinpfoertner marvinpfoertner added refactoring Refactoring of existing functionality testing and CI Unit tests, coverage and continuous integration dependencies Pull requests that update a dependency file labels Mar 27, 2022
@marvinpfoertner marvinpfoertner self-assigned this Mar 27, 2022
@marvinpfoertner marvinpfoertner requested a review from a team as a code owner March 27, 2022 12:02
@marvinpfoertner
Copy link
Collaborator Author

FYI: This PR is probably best reviewed commit by commit

marvinpfoertner
marvinpfoertner commented Mar 27, 2022
View reviewed changes
pyproject.toml
"any",
]
zip-safe = false
packages = ["probnum"]
Copy link
Collaborator Author

@marvinpfoertner marvinpfoertner Mar 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I specified probnum here manually instead of collecting it magically using the [tool.setuptools.packages.find] directive. This way, we need less configuration lines and we avoid weird artifacts if stuff that we don't want installed gets collected. "Explicit is better than implicit" 😁

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This means that any additional packages under src that we want installed must also be listed here in the future, which gives a very nice overview of the package contents.

marvinpfoertner
marvinpfoertner commented Mar 27, 2022
View reviewed changes
pyproject.toml
"any",
]
zip-safe = false
packages = ["probnum"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This means that any additional packages under src that we want installed must also be listed here in the future, which gives a very nice overview of the package contents.

pyproject.toml
use_parentheses = true
line_length = 88
# see https://pycqa.github.io/isort/docs/configuration/profiles.html#black
profile = "black"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This combines all deleted options, but it will be kept up-to-date with blacks behavior

@marvinpfoertner marvinpfoertner mentioned this pull request Mar 27, 2022
Open
@JonathanWenger JonathanWenger self-assigned this Mar 28, 2022
JonathanWenger
JonathanWenger approved these changes Mar 29, 2022
View reviewed changes
Copy link
Contributor

@JonathanWenger JonathanWenger left a comment

Choose a reason for hiding this comment

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

Looks good to me! I like the consolidation of config files. The only change I'd suggest is to not explicitly specify authors and maintainers.

pyproject.toml Outdated
readme = "README.md"
requires-python = ">=3.8,<3.11"
license = {file = "LICENSE.txt"}
authors = [
Copy link
Contributor

@JonathanWenger JonathanWenger Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I realize having specific names and e-mail addresses here is probably part of the PyPI specification, but I am not a huge fan, since it will cause PRs when maintainers change and may not stay up-to-date. (I realize similar issues exist with the docs). I somewhat prefer the option to just specify this on GitHub and let the contributors list show who creates this package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case we should create a joint mail address for all ProbNum maintainers soon, since this is the only piece of contact information listed on PyPI. It also seems to be quite common in other open source project (see NumPy, SciPy, PyTorch)

setup.py Show resolved Hide resolved
marvinpfoertner and others added 6 commits March 29, 2022 09:34
@marvinpfoertner
Remove authors and maintainers
cfe0a16
@marvinpfoertner
Bugfix: optional-dependencies is generated dynamically
4f1c0cb
@marvinpfoertner
Fix warning in build process: Add homepage URL
fa66639
@dependabot @marvinpfoertner
Update sphinx requirement from <4.5,>=3.0 to >=3.0,<4.6 (probabilisti…
0cd5215 
…c-numerics#688)
@PrSh27 @marvinpfoertner
Resolve pylint line-too-long for linops (probabilistic-numerics#682)
822a0cb 
* Remove line-to-long for _problems

* Remove

* Remove line-too-long for _scaling

* -

* Remove line-too-long from /linops and add to/problems

* Update _kronecker.py

* Fix trailing-whitespace

* Fix trailing whitespace for kroneker

* Fix trailing-whitespaces for _linear_operator
@marvinpfoertner
Merge remote-tracking branch 'origin/main' into pep-621-refactor
717e0bb
@marvinpfoertner marvinpfoertner merged commit fc32cce into probabilistic-numerics:main Mar 29, 2022
@marvinpfoertner marvinpfoertner deleted the pep-621-refactor branch March 29, 2022 09:21
nirmal-j-patel pushed a commit to nirmal-j-patel/probnum that referenced this pull request Apr 1, 2022
@marvinpfoertner @dependabot
@PrSh27 @nirmal-j-patel
Migrate setup.cfg to pyproject.toml and refactor pyproject.toml (…
7b7a648 
…probabilistic-numerics#685)

* Migrate `setup.cfg` to PEP 621 Project Metadata in `pyproject.toml`

* Add URLs in `pyproject.toml` testing config

* Update `black` configuration

* Update `isort` configuration

* Add headers for formatting and linting configuration

* Move formatting configuration section

* Rearrange linting configuration

* Add maintainers and authors

* Bugfix in `tool.setuptools.platforms`

* Bugfix in package collection

* Bugfix in `project.optional-dependencies`

* Don't need requirements.txt anymore

* One line per package dependency

* Bugfix in `[project.version]`

* Remove authors and maintainers

* Bugfix: `optional-dependencies` is generated dynamically

* Fix warning in build process: Add `homepage` URL

* Update sphinx requirement from <4.5,>=3.0 to >=3.0,<4.6 (probabilistic-numerics#688)

* Resolve pylint line-too-long for linops (probabilistic-numerics#682)

* Remove line-to-long for _problems

* Remove

* Remove line-too-long for _scaling

* -

* Remove line-too-long from /linops and add to/problems

* Update _kronecker.py

* Fix trailing-whitespace

* Fix trailing whitespace for kroneker

* Fix trailing-whitespaces for _linear_operator

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: PrSh27 <102277860+PrSh27@users.noreply.github.com>
@JonathanWenger JonathanWenger mentioned this pull request Feb 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonathanWenger JonathanWenger JonathanWenger approved these changes

Assignees

@marvinpfoertner marvinpfoertner

@JonathanWenger JonathanWenger

Labels
dependencies Pull requests that update a dependency file refactoring Refactoring of existing functionality testing and CI Unit tests, coverage and continuous integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marvinpfoertner @JonathanWenger @PrSh27