Skip to content

Run slyp against the code base #2382

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

Closed
wants to merge 1 commit into from
Closed

Conversation

kurtmckee
Copy link
Contributor

This addresses several formatting issues, including:

  • Same-line str concatenation
  • Same-indentation-level multi-line str concatenation in function parameters and dict definitions
  • Unnecessary parentheses around str constants

Although the initial work was automated, manual work was required to reflow multi-line concatenated strings to fit under the 100-character limit.

Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.16%. Comparing base (342642e) to head (c520921).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2382   +/-   ##
=======================================
  Coverage   99.16%   99.16%           
=======================================
  Files          40       40           
  Lines        3101     3101           
  Branches      680      680           
=======================================
  Hits         3075     3075           
  Misses         15       15           
  Partials       11       11           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Some of the old code reads fine to me. I don't really need parentheses for multi line strings, but that might be my personal preference.

The unnecessary concats are nice of course.

This addresses several formatting issues, including:

* Same-line str concatenation
* Same-indentation-level multi-line str concatenation
  in function parameters and dict definitions
* Unnecessary parentheses around str constants

Although the initial work was automated,
manual work was required to reflow multi-line
concatenated strings to fit under the 100-character limit.
@staticdev
Copy link
Collaborator

@kurtmckee @DanielNoord sorry for late response, for me changes look ok, but I don't see a point just running slyp if we don't integrate it in the CI.

@kurtmckee
Copy link
Contributor Author

@staticdev I'm dissatisfied that the personal time I've invested trying to get isort development on track hasn't been fruitful. I'm specifically disheartened when the feedback is "looks good" but the changes don't get merged because additional conditions/requirements are added months after I've invested my time.

I'm sufficiently dissatisfied by these interactions that I no longer want to try contributing to isort. I wish the project well, as I still intend to use it. However, I'm not motivated to invest my time toward its success and maintenance.

@kurtmckee kurtmckee closed this May 12, 2025
@kurtmckee kurtmckee deleted the run-slyp branch May 12, 2025 13:10
@staticdev
Copy link
Collaborator

Dear @kurtmckee, I understand your frustration and it is sad to me you took this decision, but also I am doing work here as best effort and had a lot of work in the last months. This MR specifically was not addressing any issue specific and I though adding to CI is necessary since you fix slyp today and next MR can break it. This is standard in any project using this kind of tools.

@kurtmckee
Copy link
Contributor Author

kurtmckee commented May 13, 2025

@staticdev I'm grateful for your continued work on isort.

You're correct that this wasn't addressing a bug or feature, but my goal wasn't to introduce slyp, only to mention what tool I used to find these improvement opportunities. Perhaps the PR title should have been "fix some code quality issues".

When I take time away from my family and other priorities to work on open source, my goal is to improve the software that I use, but what I think I'm wanting is (for lack of a more objective description) "positive vibes" for that effort, and I don't currently feel that when contributing to isort.

It's not simply this PR. Back in January I noticed that isort had completely stalled out and tried to get development started again by doing bug triage. @DanielNoord showed up and started responding, and in less than a week almost 40 issues and PRs had been addressed. However, in PyCQA/meta#64 Daniel was effectively told to stop taking action, so he did (and so did I). My coworker @sirosen offered to pick up maintenance, but that offer was rejected, too.

I've tried to contribute PRs to isort, but Daniel is still honoring that discussion in PyCQA/meta#64 and won't merge them. Save for that initial week back in January where it felt like Daniel and I were working well together, I'm not getting "positive vibes".

My impression is that isort is stagnating because the current gatekeepers at the repo and org levels are not making concrete decisions and taking action (as documented in PyCQA/meta#64). I recommend overcoming that stagnation.

And again, I'm grateful for your continued work on isort.

@sirosen
Copy link

sirosen commented May 14, 2025

👋 Hi there! Mentioned person, coworker of @kurtmckee, and author of slyp (https://slyp.readthedocs.io/en/stable/) here!

As far as I can tell, Kurt was transparent about "I used this tool to transform the code", but it was submitted in the same spirit as a manual cleanup that someone might have suggested as an interested contributor.

I vehemently disagree that not putting a tool into CI means that the changes themselves should be rejected. If someone told me "I fixed this oddity I spotted in your project which I found with PyCharm," I wouldn't decide that I can't take the change simply because we don't have "PyCharm CI".

In my experience from the maintainer side, suggesting new tools in CI without prior buy-in from maintainers is often a bad experience for the maintainers. It's better not to be changing the linting and autofixing configuration of a project where you are only a contributor unless there is an active problem you are resolving. So I think the typical netiquette here was observed, and if you want to setup slyp as a pre-commit hook, please feel free to try it out, and file bugs if it doesn't work correctly for you!


We're using and enjoying slyp, but I don't intend to push it on anyone or even actively advertise. I will say that the paren fixer, although it may take a few minutes to get used to, can help makes bug like malformed pytest parameter lists more obvious.


Anyways, I'm here if I have anything useful to add, or if folks are curious about slyp, but will try to stay out of any deeper debates. 😄

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.

4 participants