ImportFrom level cannot be optional

[thautwarm]

BPO	35001
Nosy	@brettcannon, @srittau, @serhiy-storchaka, @asottile, @thautwarm

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-10-18.19:07:38.273>
created_at = <Date 2018-10-16.18:39:56.857>
labels = ['3.8', 'type-feature', '3.7', 'docs']
title = 'ImportFrom level cannot be optional'
updated_at = <Date 2018-10-18.19:07:38.240>
user = 'https://github.com/thautwarm'

bugs.python.org fields:

activity = <Date 2018-10-18.19:07:38.240>
actor = 'brett.cannon'
assignee = 'docs@python'
closed = True
closed_date = <Date 2018-10-18.19:07:38.273>
closer = 'brett.cannon'
components = ['Documentation']
creation = <Date 2018-10-16.18:39:56.857>
creator = 'thautwarm'
dependencies = []
files = []
hgrepos = []
issue_num = 35001
keywords = []
message_count = 10.0
messages = ['327840', '327853', '327854', '327859', '327860', '327869', '327871', '327882', '327885', '327996']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'srittau', 'docs@python', 'serhiy.storchaka', 'Anthony Sottile', 'thautwarm']
pr_nums = []
priority = 'normal'
resolution = 'third party'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35001'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[thautwarm]

This issue is found from a type hinting problem:

class ImportFrom(stmt):	class ImportFrom(stmt):
    module = ...  # type: Optional[_identifier]	    module = ...  # type: Optional[_identifier]
    names = ...  # type: typing.List[alias]	    names = ...  # type: typing.List[alias]
    level = ...  # type: Optional[int]

As we can see that level here is optional, and it's strange.

I tried ast.parse on both Python 3.5/3.6, and found that None of from a import *, from .a import *, from ..a import * and other regular cases result into an ImportFrom AST whose level is None.

Then I went to Python-asdl:

cpython/Parser/Python.asdl

Line 44 in 137b063

| ImportFrom(identifier? module, alias* names, int? level)

and got

ImportFrom(identifier? module, alias* names, int? level)

It seems like a bug. To validate it, I went to

cpython/Python/ast.c

Line 3311 in 97cf082

int idx, ndots = 0;

and got

    static stmt_ty
    ast_for_import_stmt(struct compiling *c, const node *n){
       int idx, ndots = 0;
       ...
       return ImportFrom(modname, aliases, ndots, lineno, col_offset, c->c_arena);
       ...
    }

It seems that no reason for level being None.

If it's really a bug, IMO it could be also an example that type hinting helps to error detection :-)

[asottile]

It appears it has always had this bug since introduction of absolute/relative imports in f7f438b

Agree with changing this to # type: int and correcting the documentation

[asottile]

In fact, trying to use an ImportFrom without an integer level results in a ValueError:

>>> x = ast.parse('from os import path')
>>> x.body[0].level = None
>>> compile(x, '<string>', 'exec')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid integer value: None

[serhiy-storchaka]

None is not integer. This field is optional:

>>> from ast import *
>>> x = Module(body=[ImportFrom(names=[alias(name='path', asname=None)], lineno=1, col_offset=0)])
>>> compile(x, '<string>', 'exec')
<code object <module> at 0x7f73b754da00, file "<string>", line 1>

[asottile]

Hmmm, I don't think mypy has an annotation for "sometimes has an attribute" -- Optional[T] is Union[T, None] (why I tried None).

But you're right, FromImport is constructable without a level -- it seems to behave as level=0 (I guess as expected!)

[thautwarm]

Hi, Serhiy, for Python-asdl has made level able to optional, certainly we could construct an ImportFrom AST without giving level.

Moreover, this might evidence that level cannot be optional in fact:

cpython/Python/compile.c

Line 2918 in 8e73ad3

ADDOP_LOAD_CONST_NEW(c, PyLong_FromLong(s->v.ImportFrom.level));

[thautwarm]

@anthony

I don't think mypy has an annotation for "sometimes has an attribute"

Yes.. The annotation for an attribute that **maybe exist** cannot be expressed by mypy..

[serhiy-storchaka]

The level argument of ast.ImportFrom is optional. If it is not specified or None is passes, the corresponding attribute is set to the default value 0.

Type hints are not used in the stdlib. If this is a bug in mypy, this is not the proper tracker for reporting it.

[thautwarm]

Firstly, allowing to construct ImportFrom without level specified could be the result of referring Python-asdl.

Secondly, in C level, level is always an integer.

Last but not the least, when you can access level from an ImportFrom AST, it must be an integer, not None.

[brettcannon]

There aren't any type hints in the 'ast' module in Python itself, so this is an issue with typeshed. Closing as "third party".