Remove unused `__linecnt` attribute from `_sitebuiltins`

[angelolicastro]

_sitebuiltins._Printer.__setup creates an instance attribute, __linecnt, which is unused:

cpython % git grep __linecnt
Lib/_sitebuiltins.py:        self.__linecnt = len(self.__lines)

This could be used by _sitebuiltins._Printer.__repr__ in lieu of len(self.__lines):

        self.__setup()
        if len(self.__lines) <= self.MAXLINES:
            return "\n".join(self.__lines)

But because len(self.__lines) is only referenced as such, we can remove __linecnt instead.

This was introduced in d125239 but also unused then.

Linked PRs

• gh-150427: Remove unused __linecnt attribute from _sitebuiltins #150428

[picnixz]

Honestly, I don't know whether we really need a commit for a private module like that. It's borderline cosmetic. But if we don't make that change, we'll never make it. OTOH, having a single PR for removing unused variables is also not desirable...

@hugovk @vstinner In general, when it comes to unused variables, even in private modules, do we want to clean them up or not? If it's modules I maintain, I'm ok with an additional commit because it's usually better for readability.

But it would be good if we could have a stricter rule when it comes to such PRs in the devguide (namely, what is really considered cosmetic or not).

[angelolicastro]

Thanks for the feedback!

I notice that "cosmetic" seems only to be referenced in the Triage Team devguide section, which I'd not consulted.

Happy to wait for consensus on the broader policy question.

[picnixz]

Oh this is definitely an oversight. We do say in https://devguide.python.org/getting-started/pull-request-lifecycle/#making-good-commits:

Do not do cosmetic changes to unrelated code in the same commit as some feature/bugfix.

But if the entire PR is considered to be a feature because it's doing some cosmetic changes... well it could be argued that this doesn't apply.

@StanFromIreland Another section of our devguide that can be improved, namely cosmetic changes that we usually reject.

[StanFromIreland]

@StanFromIreland Another section of our devguide that can be improved, namely cosmetic changes that we usually reject.

Please open an issue, and I'll see what I can do :-)

[picnixz]

Ok, so Victor approved the PR and I'm also adding a +1 on this. I think we can accept refactors that remove unused attributes, even for very old modules that were not touched for the last 20+ years but I think this will be our limit (very small removals without risks are likely to be accepted)