Skip to content

config: improve typing#7401

Merged
bluetech merged 2 commits intopytest-dev:masterfrom
bluetech:typing-config
Jun 23, 2020
Merged

config: improve typing#7401
bluetech merged 2 commits intopytest-dev:masterfrom
bluetech:typing-config

Conversation

@bluetech
Copy link
Member

This improves the typing around _pytest.config, and also makes pytest completely clean with regards to the py typings in pytest-dev/py#232.

"getfslineno",
"getrawcode",
"Traceback",
"TracebackEntry",
Copy link
Member Author

Choose a reason for hiding this comment

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

TracebackEntry is exposed through Traceback so it should be visible too (though this is an internal package anyway).


@property
def path(self):
def path(self) -> Union[py.path.local, str]:
Copy link
Member Author

Choose a reason for hiding this comment

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

Code.path has this annoying type due to some error path, we should fix it sometime...

self._conftestpath2mod = {} # type: Dict[Any, object]
# State related to local conftest plugins.
self._dirpath2confmods = {} # type: Dict[py.path.local, List[types.ModuleType]]
self._conftestpath2mod = {} # type: Dict[Path, types.ModuleType]
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually stores Path and not what the previous comment said.

I removed the comments since they are redundant with the types.

assert isinstance(modname, str), (
"module name as text required, got %r" % modname
)
modname = str(modname)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed per assert above.

"""

args = attr.ib(converter=tuple)
args = attr.ib(type=Tuple[str, ...], converter=_args_converter)
Copy link
Member Author

Choose a reason for hiding this comment

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

Explicit converter instead of tuple needed just for the type annotation to work


def _strtobool(val):
"""Convert a string representation of truth to true (1) or false (0).
def _strtobool(val: str) -> bool:
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed _strtobool to return bool instead of 0/1, seems more sensible.


if auto_indent_option is None:
return 0
elif type(auto_indent_option) is int:
Copy link
Member Author

Choose a reason for hiding this comment

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

mypy doesn't do type narrowing on type(...) is .., so I changed the code to use isinstance instead. Then things need to move a bit because isinstance(True, int) is true.

raise ConftestImportFailure(path, sys.exc_info()) from e
except Exception as exc:
assert exc.__traceback__ is not None
exc_info = (type(exc), exc, exc.__traceback__)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed because sys.exc_info() has types with BaseException while we want just Exception.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the hardworking here!



def _get_plugin_specs_as_list(specs):
def _get_plugin_specs_as_list(specs) -> List[str]:
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious, don't we need to type specs here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added another commit which does this now, but it also refactors the function a bit. I was thrown off by the ModuleType check but found the reference issue.

@bluetech bluetech merged commit 3624acb into pytest-dev:master Jun 23, 2020
@bluetech bluetech deleted the typing-config branch June 28, 2020 15:12
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.

2 participants