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

Possible use for pass #2642

Open
Day0Dreamer opened this issue May 4, 2023 · 8 comments
Open

Possible use for pass #2642

Day0Dreamer opened this issue May 4, 2023 · 8 comments

Comments

@Day0Dreamer
Copy link

Day0Dreamer commented May 4, 2023

From:
WPS420 — Forbid some python keywords.

pass keyword is just useless by design. There’s no use-case for it. Because it does literally nothing.

This is a snippet from a selector. In short, we need at the end a copy of render_data. We can get render_data out of doc, but sometimes we have only the data and no doc in sight.

class RenderSettings(object):

    def __init__(
        self,
        doc: Optional[c4d.documents.BaseDocument] | None = None,
        render_data: Optional[RenderData] | None = None,
    ):
        match (doc, render_data):
            case (None, None):
                doc = c4d.documents.GetActiveDocument()
                render_data = doc.GetActiveRenderData()
            case (None, _):
                pass
            case (_, None):
                render_data = doc.GetActiveRenderData()
            case (_, _):
                raise ValueError("doc and render_data are mutually exclusive.")

        if render_data is None:
            raise RuntimeError("No render data found.")

        self.rd: RenderData = render_data.GetClone()

It would seem, I literally want to just exit the match-case in a case where we have only render_data at the input.

Am I missing something?

@sobolevn
Copy link
Member

sobolevn commented May 4, 2023

Hm, interesting.

The other way would be:

>>> match (None, 2):
...    case (x, _) if x is not None:
...        raise ValueError
... 

But, I agree that pass seems like the clearest solution here.

@kehrazy
Copy link

kehrazy commented May 5, 2023

Do we really have to make our match statements inclusive? Is there a need for
case (None, _): pass
at all?

It wouldn't match - the code wouldn't execute - basically the pass statement. Am I wrong here?

@Day0Dreamer
Copy link
Author

Day0Dreamer commented May 5, 2023

@kehrazy I'd spend more time remembering and concluding why are there 3 cases for a problem with 2^2=4 possible solutions, than reading the pass case.

This feels like the case about declaring bit masks for filters alike firewall works

class PluginsFilterBits(object):
    """A class for filter bits."""

    known_to_renderfarm = FilterBit(bit=0b0100, name="known_to_renderfarm")             # noqa: WPS339
    built_in = FilterBit(bit=0b0010, name="built_in")                                   # noqa: WPS339
    is_script = FilterBit(bit=0b0001, name="is_script")                                 # noqa: WPS339
    weird = FilterBit(bit=0b1000, name="weird")                                         # noqa: WPS339

It is useful for when debugging and mentally reiterating "This case works, this case works,, this case works,, this case works,, this case works, how many cases were there? Eight? Okay, I've checked all eight"

Or with the bitmask - having it always in full length, with trailing zeroes, is in terms of Chat GPT, tokenizible. Meaning I can have for each bitmask remember it's feeling and operate with it, rather than pure logic.

@kehrazy
Copy link

kehrazy commented May 7, 2023

@Day0Dreamer hey, yeah, I can see the problem here, thanks for pointing this out. However, personally, when looking at this particular function, my brain goes "Yeah, this function handles these particular cases this way" - not "So, 2^2 - 4 cases... Huh, yeah, there's one that's not covered/handled here - should it be?"

You can add a comment, after all.

And yeah, I know about the "explicit is better than implicit" thing - does this apply here?

@sobolevn
Copy link
Member

sobolevn commented May 7, 2023

pass here means exactly what it should do in the first place: nothing.
So, providing pass in a case statement means that this case does not do anything.

As far as I know, there's no other way to describe the same thing without pass and using ... here is not correct.

@kehrazy
Copy link

kehrazy commented May 7, 2023

Why would we even declare a pass?

def verify_some_things(value_to_verify: int) -> bool:
    """
    This function verifies a lot of cool data.

    .. code:: python

        >>> verify_some_things(2)
        this is a two!
        True

    :param value_to_verify: an integer, the data to be verified.
    :return: bool - True if successful, False if not.
    """
    
    match value_to_verify:
        case 1:
            print("this is a one!")
            return True
        case 2:
            print("this is a two!")
            return True

    return False


if __name__ == "__main__":
    verify_some_things(1)  # this is a two!
    verify_some_things(2)  # this is a two!
    verify_some_things(3)  # prints nothing, there's no values matching.

@Day0Dreamer
Copy link
Author

@kehrazy this comes from a habit of making a truth tables, where each state is accounted for. From the field of general computer science and electronics.

It's quite common while designing a logic machine, to start with a truth table, that describes all possible states.

Overall, it's a human construct to help humans not forget stuff and make debugging more structured.

What I think is actionable about this thread – documentation update. I really use a lot of the sections "This is a wrong way, this is a right way, this is the exception to the rule" inside the docs.

p.s. — Already added a noqa comment and forgot about the problem :D

@webknjaz
Copy link
Contributor

I've got another use-case for pass to add:

try:
    some_data = find_a_thing()
except LookupError:
    pass
else:
    some_data.do_stuff()

Of course, pass could become ... but the latter generates an op-code, while the former does not. It could be made a ("doc")string which has the same weakness and would additionally generate extra linting violations and be generally misplaced.

Doing

with contextlib.suppress(LookupError):
    some_data = find_a_thing()
    some_data.do_stuff()

is also a no-go since it has the same problem as several instructions in a try-block — it's unclear which line is expected to generate an exception and the behavior may silently regress over time.

Following the try-except-else idiom makes things clear, allowing me to communicate the intent. Using contextlib.suppress() is fine for single-line suppressions, but not for this case.

This all is to say that the assumption WPS420 makes regarding pass and the claim that it's useless is not universally true. It shouldn't be emitted in for try-except blocks that have an else part.

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

No branches or pull requests

4 participants