Skip to content

config: typing for create_terminal_writer, re-export TerminalWriter #6545

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

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jan 23, 2020

This also imports TerminalWriter explicitly, allowing for easier
phasing out / replacing of it (via the import, which then could go
through the _pytest.compat module).

Note to self:

pick b34f07360 config: typing for create_terminal_writer
pick 443335426 Import TerminalWriter from `py.io`

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

LGTM except the unrelated bit.

@blueyed
Copy link
Contributor Author

blueyed commented Jan 23, 2020

I wonder if I should import it via _pytest.compat here already, so that there's really a single place where to extend/change it, and/or put it into _pytest._io in the first place - which would be just the import for now, but could later extend the class, i.e. to support "underline" markup?

@bluetech
Copy link
Member

Importing from _pytest._io makes sense to me.

@blueyed
Copy link
Contributor Author

blueyed commented Jan 23, 2020

Importing from _pytest._io makes sense to me.

Moved it there. Not sure if it should use __all__, explicit # noqa: F401 etc though.

@bluetech
Copy link
Member

Mypy has a convention (a bit more than a convention actually, given the --no-implicit-reexport flag) where reexports are done like

from py.io import TerminalWriter as TerminalWriter

I don't know how flake8 would like it, but I'd do it like this with a comment explaining anyway:

# Reexport TerminalWriter from here instead of py, to make it easier to
# extend or swap our own implementation in the future.
from py.io import TerminalWriter as TerminalWriter  # noqa: F<something>

@blueyed
Copy link
Contributor Author

blueyed commented Jan 23, 2020

Thanks, amended:

diff --git c/src/_pytest/_io/__init__.py i/src/_pytest/_io/__init__.py
index 6265353d8..047bb179a 100644
--- c/src/_pytest/_io/__init__.py
+++ i/src/_pytest/_io/__init__.py
@@ -1 +1,3 @@
-from py.io import TerminalWriter
+# Reexport TerminalWriter from here instead of py, to make it easier to
+# extend or swap our own implementation in the future.
+from py.io import TerminalWriter as TerminalWriter  # noqa: F401
diff --git c/tox.ini i/tox.ini
index fae23b882..707f239d0 100644
--- c/tox.ini
+++ i/tox.ini
@@ -189,8 +189,6 @@ markers =
 [flake8]
 max-line-length = 120
 extend-ignore = E203
-per-file-ignores =
-  src/_pytest/_io/__init__.py: F401

 [isort]
 ; This config mimics what reorder-python-imports does.

@blueyed blueyed changed the title config: typing for create_terminal_writer config: typing for create_terminal_writer, re-export TerminalWriter Jan 23, 2020
This also imports `TerminalWriter` explicitly via `_pytest._io`,
allowing for easier extending / replacing it.
@blueyed blueyed merged commit 8ca8d25 into pytest-dev:master Jan 23, 2020
@blueyed blueyed deleted the terminalwriter branch January 23, 2020 14:42
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