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

ssort messes with class attribute docstrings #70

Open
and-semakin opened this issue Mar 29, 2022 · 2 comments
Open

ssort messes with class attribute docstrings #70

and-semakin opened this issue Mar 29, 2022 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@and-semakin
Copy link

Before:

from dataclasses import dataclass


@dataclass
class User:
    id: int
    """User identifier."""

    email: str
    """E-mail address."""

    phone: str
    """Phone number in the international format."""

    age: int
    """Number of full years."""

After:

from dataclasses import dataclass


@dataclass
class User:
    id: int

    email: str

    phone: str

    age: int
    """User identifier."""
    """E-mail address."""
    """Phone number in the international format."""
    """Number of full years."""

There are two PEPs that describe such docstrings. Both of them were rejected.

I know that this is now a "real" docstring syntax but people still use it to document fields in their classes. Some tools support it as well (like sphinx).

It could be useful to recognise such docstrings and preserve their positions.

@bwhmather
Copy link
Owner

Interesting. I wasn't aware of that convention. It's always bugged me that documentation comments appear before the things they describe in code but after in the docs.

I don't think this conflicts with any of the other rules for how we sort class bodies so I don't see any harm in implementing it. With that said, it is unlikely to be a straightforward change as we have no precedent for pairing expressions with siblings.

I don't have the time to work on this myself, but I'm going to leave this open for you or anyone else to pick up. A PR which implements this behaviour without breaking anything else or tanking performance will be accepted. Happy to provide advice and however many reviews it takes to get something working.

In the interim, switching to #: comments is probably the only workaround.

(Linking to conversation in #11)

@bwhmather bwhmather added enhancement New feature or request good first issue Good for newcomers labels Mar 29, 2022
KyleKing added a commit to KyleKing/pytest_cache_assert that referenced this issue Sep 26, 2022
KyleKing added a commit to KyleKing/pytest_cache_assert that referenced this issue Sep 26, 2022
@pawamoy
Copy link

pawamoy commented Oct 7, 2023

Hi @bwhmather, I don't think I have the time to work on this, but could give us some hints where/how this would be implemented, just in case? A quick overview of ssort's flow, with the corresponding modules holding the logic, would be a great start!

pawamoy added a commit to pawamoy/copier-pdm that referenced this issue Oct 7, 2023
`ssort` re-orders class attributes without taking
their docstrings into account.
See bwhmather/ssort#70
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants