Skip to content

Conversation

XzzX
Copy link
Contributor

@XzzX XzzX commented Dec 18, 2024

Waiting for
pyiron/actions#138
and
#520

@XzzX XzzX requested a review from liamhuber December 18, 2024 08:49
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/black

@liamhuber
Copy link
Member

I'm sick and my kid's school hasn't restarted yet, so I'm not going to get to resolving the conflicts yet. Easier will probably be to just cherry pick the top commit and then re-black.

Per your comment elsewhere (sorry, lost the ref), if black is going to complain about the multi-line strings, it might be time to just give up on that, despite the fact we both find our version somewhat more readable. The silver lining would be that we could abandon black entirely and just use the ruff formatting...

Copy link

codacy-production bot commented Jan 6, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (402b3d2) 3356 3070 91.48%
Head commit (77110a1) 3356 (+0) 3070 (+0) 91.48% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#524) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12640274584

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.478%

Totals Coverage Status
Change from base Build 12638731071: 0.0%
Covered Lines: 3070
Relevant Lines: 3356

💛 - Coveralls

@liamhuber
Copy link
Member

liamhuber commented Jan 6, 2025

Ah, sorry @XzzX, I tried to roll back head and merge main so that your commit touching pyproject.toml stayed intact, but evidently I failed. I don't feel it's a big enough deal to go back and try all over again, but just FYI I was trying to keep you credited for the change in the git history.

We need to find a way how to deal with multiline strings also in black.

I'm ready to concede to the formatters on this this one. If ruff and black are both pushing the same thing, I don't have enough willpower to fight it. Are you ok with their slightly-less-readable strings?

I was briefly excited because they were both ok with the following:

        self.assertLess(
            dt,
            1.1 * t,
            msg="""
                Expected the sleeps to run in parallel with minimal overhead (since 
                it's just a thread pool executor) -- the advantage is that the 
                constructors should survive (de)serialization
            """,
        )

(Also as f"""-strings.) But, while this is perfectly readable from the code perspective, it actually prints the error message with all the extra indentation 🤦‍♂️

@liamhuber liamhuber removed their request for review January 6, 2025 21:09
@XzzX
Copy link
Contributor Author

XzzX commented Jan 7, 2025

Ah, sorry @XzzX, I tried to roll back head and merge main so that your commit touching pyproject.toml stayed intact, but evidently I failed. I don't feel it's a big enough deal to go back and try all over again, but just FYI I was trying to keep you credited for the change in the git history.

We need to find a way how to deal with multiline strings also in black.

I'm ready to concede to the formatters on this this one. If ruff and black are both pushing the same thing, I don't have enough willpower to fight it. Are you ok with their slightly-less-readable strings?

I was briefly excited because they were both ok with the following:

        self.assertLess(
            dt,
            1.1 * t,
            msg="""
                Expected the sleeps to run in parallel with minimal overhead (since 
                it's just a thread pool executor) -- the advantage is that the 
                constructors should survive (de)serialization
            """,
        )

(Also as f"""-strings.) But, while this is perfectly readable from the code perspective, it actually prints the error message with all the extra indentation 🤦‍♂️

Thank you, kind of you trying to preserve the history.
I think having a consistent formatting is more important than my personal preference. So I am ok with it.

@liamhuber liamhuber merged commit 6aed9fe into main Jan 7, 2025
21 checks passed
@liamhuber liamhuber deleted the black branch January 7, 2025 15:29
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.

3 participants