bpo-39999: Improve compatibility of the ast module.#19056
bpo-39999: Improve compatibility of the ast module.#19056serhiy-storchaka merged 5 commits intopython:masterfrom
Conversation
* Re-add removed classes Suite, Param, AugLoad and AugStore. * Add docstrings for dummy classes. * Add docstrings for attribute aliases. * Set __module__ to "ast" instead of "_ast".
|
|
||
| class Suite(mod): | ||
| """Unused AST node class.""" | ||
|
|
||
| class AugLoad(expr_context): | ||
| """Unused AST node class.""" | ||
|
|
||
| class AugStore(expr_context): | ||
| """Unused AST node class.""" | ||
|
|
||
| class Param(expr_context): | ||
| """Unused AST node class""" |
There was a problem hiding this comment.
Should we deprecate this as well?
There was a problem hiding this comment.
I used word "deprecated" if it was used in past Python releases and the user code can use them for compatibility. I used word "unused" if it was not used in Python (at least in Python 3) and there is no reason to use it in the user code.
There was a problem hiding this comment.
But aren't them will be removed in time or stay them forever? If they will, wouldn't it be good to start doing warnings?
There was a problem hiding this comment.
It would be better to start emitting warnings when the user code can just drop these classes. Currently Param is needed for 2.7, and some projects still support it.
There was a problem hiding this comment.
Currently Param is needed for 2.7, and some projects still support it.
Oh, that makes sense.
| Constant.n = property(_getter, _setter) | ||
| Constant.s = property(_getter, _setter) | ||
| Constant.n = property(_getter, _setter, doc='Deprecated. Use value instead.') | ||
| Constant.s = property(_getter, _setter, doc='Deprecated. Use value instead.') |
There was a problem hiding this comment.
Would it be possible to emit a DeprecationWarning when it's used?
There was a problem hiding this comment.
Yes, it is possible. But it would be better to start emitting warnings when the user code no longer need to use the deprecated feature. I.e. when the support of Python 3.7 be dropped in most projects which use AST.
| } | ||
|
|
||
| class Index(AST): | ||
| class slice(AST): |
There was a problem hiding this comment.
@serhiy-storchaka what do you think about implementing custom instance and subclass checks for slice? slice is a sum type, so there should be checks like this issubclass(thing, ast.slice) or isinstance(thing, ast.slice)
something like this should work
def __subclasscheck__(self, subclass):
if issubclass(subclass, Slice):
return True
else:
return super().__subclasscheck__(subclass)
def __instancecheck__(self, instance):
if isinstance(instance, Slice):
return True
else:
return super().__instancecheck__(instance)There was a problem hiding this comment.
Before the simplification, it was part of slice kind. Now it is an expression by it is own but if we want to increase compatibility, wouldn't it better to it behave like a subclass of ast.slice?
There was a problem hiding this comment.
I do not think it would be useful because Index() can return arbitrary node type and it is impossible to distinguish ExtSlice from Tuple.
* Re-add removed classes Suite, slice, Param, AugLoad and AugStore. * Add docstrings for dummy classes. * Add docstrings for attribute aliases. * Set __module__ to "ast" instead of "_ast".
__module__to "ast" instead of "_ast".https://bugs.python.org/issue39999