Skip to content

Use pyproject-fmt instead of pretty-format-toml - #1868

Merged
namurphy merged 3 commits into
PlasmaPy:mainfrom
namurphy:pyproject-fmt
Jan 4, 2023
Merged

Use pyproject-fmt instead of pretty-format-toml#1868
namurphy merged 3 commits into
PlasmaPy:mainfrom
namurphy:pyproject-fmt

Conversation

@namurphy

@namurphy namurphy commented Jan 4, 2023

Copy link
Copy Markdown
Member

Description

This PR adds a pre-commit hook using pyproject-fmt and drops the pre-commit hook using pretty-format-toml.

Motivation and context

It looks like the only TOML file that we have is our pyproject.toml, so it makes sense to use a tool that has more specific and standardized formatting for pyproject.tomls. (Or is it pyprojects.toml? I don't know what the plural of pyproject.toml is.)

We're also running into a problem with our pre-commit config due to a new inconsistency upstream of pretty-format-toml. Removing pretty-format-toml will address this problem (though that issue will probably get fixed in a few days anyway).

This time I did not hear about this tool from a podcast! I read about it in the show notes of a podcast instead. 😬

@namurphy namurphy added the no changelog entry needed For PRs that should not have a changelog entry label Jan 4, 2023
@github-actions

github-actions Bot commented Jan 4, 2023

Copy link
Copy Markdown

Thank you for contributing to PlasmaPy! The project's future depends deeply on contributors like you, so we deeply appreciate it! 🌱 The following checklist will be used by the code reviewer to help guide the code review process.

  • Overall
    • Does the PR do what it intends to do?
    • Except for very minor changes, is a changelog entry included and consistent with the changelog guide?
    • Are the continuous integration checks passing? (Most linter problems can be automagically fixed by commenting on this PR with pre-commit.ci autofix.)
  • Code
    • Is new/updated code understandable and consistent with the coding guide?
    • Are there ways to greatly simplify the implementation?
    • Are there any large functions that should be split up into shorter functions?
  • Tests
    • Are tests added/updated as required, and consistent with the testing guide?
    • Are the tests understandable?
    • Do the tests cover all important cases?
  • Docs
    • Are docs added/updated as required, and consistent with the doc guide?
    • Are the docs understandable?
    • Do the docs show up correctly in the preview, including Jupyter notebooks?

@namurphy
namurphy marked this pull request as ready for review January 4, 2023 18:50
@namurphy
namurphy requested a review from a team as a code owner January 4, 2023 18:50
@namurphy
namurphy requested review from StanczakDominik and removed request for a team January 4, 2023 18:50
@codecov

codecov Bot commented Jan 4, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 98.31% // Head: 98.31% // No change to project coverage 👍

Coverage data is based on head (d55137e) compared to base (a970130).
Patch has no changes to coverable lines.

❗ Current head d55137e differs from pull request most recent head 97b0680. Consider uploading reports for the commit 97b0680 to get more accurate results

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1868   +/-   ##
=======================================
  Coverage   98.31%   98.31%           
=======================================
  Files          95       95           
  Lines        8372     8372           
=======================================
  Hits         8231     8231           
  Misses        141      141

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

StanczakDominik
StanczakDominik approved these changes Jan 4, 2023
View reviewed changes
Comment thread pyproject.toml Resolved
@namurphy

namurphy commented Jan 4, 2023

Copy link
Copy Markdown
Member Author

Thank you for the quick review! Happy to get this in to make pre-commit happy.

@namurphy @StanczakDominik
Remove outdated comment
97b0680 
Co-authored-by: Dominik Stańczak-Marikin <[email protected]>
@namurphy
namurphy enabled auto-merge (squash) January 4, 2023 21:07
@namurphy
namurphy merged commit 25e8776 into PlasmaPy:main Jan 4, 2023
@namurphy
namurphy deleted the pyproject-fmt branch May 24, 2023 16:04
@nstarman nstarman mentioned this pull request Apr 18, 2024
Closed
@namurphy

namurphy commented Apr 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

It appears we no longer use pyproject-fmt, though I wasn't able to find where we removed it. I just tried adding it again, and it in addition to sorting, it also removed comment-only lines (tox-dev/pyproject-fmt#16), which blocks us from using it. (For example, we want to keep in comments that say why a particular dependency is pinned, and sometimes need multiple lines to do that.)

@namurphy namurphy mentioned this pull request Mar 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StanczakDominik StanczakDominik StanczakDominik approved these changes

Assignees

No one assigned

Labels

no changelog entry needed For PRs that should not have a changelog entry

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@namurphy @StanczakDominik