Use collections.abc instead of deprecated typing for iterables objects#7329
Use collections.abc instead of deprecated typing for iterables objects#7329geooo109 merged 17 commits intotobymao:mainfrom
Conversation
- replaced all deprecated typing.{Iterable, Iterator, etc...} uses to correspond collections.abc types
|
@OutSquareCapital thanks for the PR. What was the motivation behind this ? it was just to remove There are places that we can't avoid |
|
It's a first step for further typing contributions. I have 2 other branches following (1 ready already). |
- replaced all deprecated typing.{Iterable, Iterator, etc...} uses to correspond collections.abc types
…ital/sqlglot into typing-improvements
| return self | ||
|
|
||
| def error_messages(self, args: t.Optional[t.Sequence] = None) -> t.List[str]: | ||
| def error_messages(self, args: t.Optional[Sequence[object]] = None) -> list[str]: |
There was a problem hiding this comment.
Why this is an object ?
There was a problem hiding this comment.
unnanotated generics objects wil default to typing.Any.
It won't be an error at the caller site, but will be at the definition site for stricter type checkers (pylance, basedpyright for example).
object is always preferrable to typing.Any for various reasons:
- same "this can be anything" permissivity for most cases
- once you start trying to access some specific attributes on a variable
x, e.glen(x),xbeing typed asobjectwill flag this as an error.
This can give the opportunity to narrow the type, which is always preferable than keeping it asAny. - no import (no pertinent here)
Any is almost never needed.
You can always use type Union, generics or Protocols instead.
But since this PR was already pretty big I keeped it simple and just defaulted to "if unnanotated, use object if it works, else typing.Any"
There was a problem hiding this comment.
@OutSquareCapital I see ok, let's use the any logic for now and match the previous approach because we do a lot of changes at once. (so keep the previous approach but use Sequence instead of t.Sequence as we do for the rest of the PR). In a follow work we can focus on these cases.
sqlglot/optimizer/scope.py
Outdated
| return walk_in_scope(self.expression, prune=prune) | ||
|
|
||
| def find(self, *expression_types: t.Type[E]) -> t.Optional[E]: | ||
| def find(self, *expression_types: type[E]) -> t.Optional[E]: |
There was a problem hiding this comment.
For these cases can we import also the from builtins import type as Type and use it ? in order to be robust for all the codebase. So, everywhere we should have Type instead of t.Type or type
There was a problem hiding this comment.
Everywhere EVERYWHERE or everywhere in my current changes? The former will be easier to do but will up by a lot the nb of files impacted
There was a problem hiding this comment.
@OutSquareCapital only in the current files you changed.
There was a problem hiding this comment.
Done!
I changed a few more that still used t.Type (in the same already impacted files OFC).
|
@OutSquareCapital most of the changes LGTM, left some comments. |
|
@OutSquareCapital when you resolve both my comments, we are ready to merge. |
2 participants
typing.{Set, Dict, List, Tuple, Iterator, Iterable, etc...}is deprecated in 3.9.No change wathsoever outside of pure typing constructs, so no runtime impact at all.
I originally wanted to do a full modernization with
list/dict/set/tupleinstead of theirtypingcounterparts, but it was becoming way too big of a change.Instead I changed some of them who were around my
collections.abcchanges.EDIT:
sorry for the numerous workflow reruns. I hadn't understood immediatly that it would reject unformatted code. I tought that it would format it as part of the workflow.