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

Followup changes to fix ruff & pyright warnings #203

Merged
merged 6 commits into from
Feb 22, 2024

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Feb 21, 2024

  • test_flake8_trio now subclasses Statement to add __eq__, so it's properly hashable elsewhere.
  • error_codes is now annotated as Mapping, silencing RUF012

Barely needs much of a review. @thejcannon @aneeshusa

… about mutable classvar, subclass Statement in test_flake8_trio so the original Statement is properly hashable.
)

def __hash__(self) -> int:
raise NotImplementedError

Choose a reason for hiding this comment

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

Why not super().__hash__()?

Copy link
Member Author

@jakkdl jakkdl Feb 21, 2024

Choose a reason for hiding this comment

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

That could cause objects that are equal to have different hashes (because -1 is a wildcard value), which is Not Allowed ™️
https://docs.python.org/3/reference/datamodel.html#object.__hash__

Rereading it now, it turns out I misread that originally and I didn't have to complicate things as much as I did. I originally read it as __eq__ objects had to have the same hash, and objects with same hash value had to be equal to eachother...

So I could implement __hash__() with return hash(tuple(self.name, self.lineno))

@@ -50,6 +50,22 @@
class ParseError(Exception): ...


class ColumnAgnosticStatement(Statement):
def __eq__(self, other: object) -> bool:

Choose a reason for hiding this comment

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

Probably worth a comment explaining what this class is, and why we override __eq__ and __hash__ (and what the semantics of this class are).

Copy link
Member Author

Choose a reason for hiding this comment

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

ended up reverting stuff and implementing a simple __hash__ :)

@jakkdl jakkdl merged commit bae0a63 into python-trio:main Feb 22, 2024
9 checks passed
@jakkdl jakkdl deleted the moar_ci branch February 22, 2024 09:06
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.

3 participants