refactor logging: Or assignment instead of if-else blocks

[isidroas]

In the file `Lib/logging/__init__.py` I found this pattern

if possible_false:
    target = possible_false
else:
    target = default_value

and thought it can be refactorized to

target = possible_false or default_value

in order to reduce source code space and increase human comprehension speed.

Pitfalls

• Harder to measure coverage (source https://docs.astral.sh/ruff/rules/if-else-block-instead-of-if-exp/#known-issues )

• This unnecessary change could cause conflicts in other PRs opened changing the same line of code. However,

  # source: https://stackoverflow.com/a/78905336/16926605
  $ gh pr list --state all --repo python/cpython --json number,title,files,url | jq 'map(select(.files | any(.path | contains("logging"))) | {number, title,url})'
  [
    {
      "number": 129681,
      "title": "gh-129268: Don't attempt to cleanup handlers again if logging.shutdown is called multiple times",
      "url": "https://github.com/python/cpython/pull/129681"
    }
  ]

  this is the only PR that touches the same file. It does not change the same region.

How was found

details

import sys
import ast
from typing import Union

def match_if(node: ast.If):
    # Exagerated use of assignment expressions for fun and experimenting
    if (
        len(node.body) == 1
        and isinstance(assign_if := node.body[0], ast.Assign)
        and len(targets := assign_if.targets) == 1
        and len(orelse := node.orelse) == 1
        and isinstance(assign_else := orelse[0], ast.Assign)
        ### expensive, so last:
        and compare_ast(assign_if.value, possible_false := node.test)
        and compare_ast(
            target := targets[0], assign_else.targets[0]
        )
    ):
        default_value = assign_else.value
        return (target, possible_false, default_value)

def compare_ast(
    node1: Union[ast.expr, list[ast.expr]], node2: Union[ast.expr, list[ast.expr]]
) -> bool:
    "https://stackoverflow.com/a/66733795/16926605"
    if type(node1) is not type(node2):
        return False

    if isinstance(node1, ast.AST):
        for k, v in vars(node1).items():
            if k in {"lineno", "end_lineno", "col_offset", "end_col_offset", "ctx"}:
                continue
            if not compare_ast(v, getattr(node2, k)):
                return False
        return True

    elif isinstance(node1, list) and isinstance(node2, list):
        return all(compare_ast(n1, n2) for n1, n2 in zip_longest(node1, node2))
    else:
        return node1 == node2

class IfAssignmentFinder(ast.NodeVisitor):
    def visit_If(self, node):
        if match := match_if(node):
            target, possible_false, default_value = match
            msg = "found possible refactor: %s = %s or %s" % tuple(
                ast.unparse(node) for node in [target, possible_false, default_value]
            )
            print('%d:%d:%s' % (node.lineno, node.col_offset , msg))
        else:
            self.generic_visit(node)

with open(sys.argv[1]) as f:
    tree = ast.parse(f.read())
    IfAssignmentFinder().visit(tree)

$ python my_linter.py Lib/logging/__init__.py
744:8:found possible refactor: self.linefmt = linefmt or _defaultFormatter
994:8:found possible refactor: fmt = self.formatter or _defaultFormatter

Related: https://github.com/MartinThoma/flake8-simplify?tab=readme-ov-file#sim108

[ghost]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[Wulian233]

This should merely be a change in code style. Generally it might not be accepted, but it's worth considering others' opinions

[vsajip]

I'm not very keen to make these kinds of changes. It's great that people want to contribute - why not filter open issues by the "easy" label, and pick one to work on? They will be more impactful than this type of thing. I get that it's your first CPython PR @isidroas and you may not be sure of yourself, but give it a go! The community is friendly and you will get guidance, especially if you ask on discuss.python.org if you're not sure about something. The type of change in this PR, while perhaps technically an improvement, doesn't really move the needle at all compared to how much time is spent on it.

[isidroas]

Thank you for your reply @vsajip ! I expected anything more than silence. I didn't know about that easy label, it sound that it is suitable for me.

[Wulian233]

easy

You can select labels on the issue page, but there might be a few that have already PR on by others

[isidroas]

I see, the new ones do not take long time to have a PR 💨