MNT: Do not use IPython stream in console #10942

pllim · 2020-10-26T19:56:04Z

Description

I ran into a deprecation warning when I ran pytest -s astropy/utils/tests/test_console.py:

===== test session starts =====
platform linux -- Python 3.8.3, pytest-6.1.1, py-1.9.0, pluggy-0.13.1

Running tests with Astropy version 4.2.dev1143+g094254c55.
Running tests in astropy/utils/tests/test_console.py.

Date: 2020-10-26T15:37:25

Platform: Linux-3.10.0-1160.el7.x86_64-x86_64-with-glibc2.10

Executable: .../miniconda/envs/py38/bin/python

Full Python Version: 
3.8.3 (default, May 19 2020, 18:47:26) 
[GCC 7.3.0]

encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15

Package versions: 
Numpy: 1.19.1
Scipy: 1.5.1
Matplotlib: 3.3.1
h5py: 2.10.0
Pandas: 1.0.5
PyERFA: 1.7.0
Cython: not available
Scikit-image: 0.17.2
asdf: 2.6.0

Using Astropy options: remote_data: none.

rootdir: ..., configfile: setup.cfg
plugins: hypothesis-5.16.1, openfiles-0.5.0, remotedata-0.3.1, arraydiff-0.3, asdf-2.6.0, astropy-header-0.1.2,
doctestplus-0.8.0, filter-subpackage-0.1.1, cov-2.10.1, forked-1.3.0, xdist-2.1.0
collected 28 items

astropy/utils/tests/test_console.py ..foo
foobar
...überbær
.foo
..FFFFFF..............

========== FAILURES ===========
_____ test_progress_bar _____

    def test_progress_bar():
        # This stuff is hard to test, at least smoke test it
>       with console.ProgressBar(50) as bar:

astropy/utils/tests/test_console.py:126: 
_ _ _ _ _
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
_______ test_progress_bar2 _____

    def test_progress_bar2():
>       for x in console.ProgressBar(range(50)):

astropy/utils/tests/test_console.py:132: 
_ _ _ _ _
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
_____ test_progress_bar3 _____

    def test_progress_bar3():
        def do_nothing(*args, **kwargs):
            pass
    
>       console.ProgressBar.map(do_nothing, range(50))

astropy/utils/tests/test_console.py:140: 
_ _ _ _ _
astropy/utils/console.py:752: in map
    results = cls.map_unordered(
astropy/utils/console.py:825: in map_unordered
    with cls(len(items), ipython_widget=ipython_widget, file=file) as bar:
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
_____ test_zero_progress_bar ______

    def test_zero_progress_bar():
>       with console.ProgressBar(0) as bar:

astropy/utils/tests/test_console.py:144: 
_ _ _ _ _
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
_____ test_progress_bar_as_generator _____

    def test_progress_bar_as_generator():
        sum = 0
>       for x in console.ProgressBar(range(50)):

astropy/utils/tests/test_console.py:150: 
_ _ _ _ _
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
______ test_progress_bar_map ______

    def test_progress_bar_map():
        items = list(range(100))
>       result = console.ProgressBar.map(test_progress_bar_func.func,
                                         items, step=10, multiprocess=True)

astropy/utils/tests/test_console.py:162: 
_ _ _ _ _
astropy/utils/console.py:752: in map
    results = cls.map_unordered(
astropy/utils/console.py:825: in map_unordered
    with cls(len(items), ipython_widget=ipython_widget, file=file) as bar:
astropy/utils/console.py:571: in __init__
    self.update(0)
astropy/utils/console.py:616: in update
    self._update_console(value)
astropy/utils/console.py:636: in _update_console
    write('\r|')
_ _ _ _ _

self = IPython.utils.io.IOStream(<_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>), data = '\r|'

    def write(self,data):
>       warn('IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead',
             DeprecationWarning, stacklevel=2)
E       DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead

.../IPython/utils/io.py:52: DeprecationWarning
===== short test summary info =====
FAILED astropy/utils/tests/test_console.py::test_progress_bar - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
FAILED astropy/utils/tests/test_console.py::test_progress_bar2 - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
FAILED astropy/utils/tests/test_console.py::test_progress_bar3 - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
FAILED astropy/utils/tests/test_console.py::test_zero_progress_bar - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
FAILED astropy/utils/tests/test_console.py::test_progress_bar_as_generator - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
FAILED astropy/utils/tests/test_console.py::test_progress_bar_map - DeprecationWarning: IOStream is deprecated since IPython 5.0, use sys.{stdin,stdout,stderr} instead
===== 6 failed, 22 passed in 0.98s ======

With this patch, the tests pass again:

astropy/utils/tests/test_console.py ..foo
foobar
...überbær
.foo
|=============================================================================================================================================================================|  50 / 50  (100.00%)         0s
|=============================================================================================================================================================================|  50 / 50  (100.00%)         0s
|=============================================================================================================================================================================|  50 / 50  (100.00%)         0s
|=============================================================================================================================================================================|   0 /  0  (100.00%)         0s
|=============================================================================================================================================================================|  50 / 50  (100.00%)         0s
|=============================================================================================================================================================================|  50 / 50  (100.00%)         0s
|=============================================================================================================================================================================| 100 /100  (100.00%)         0s
|=============================================================================================================================================================================| 100 /100  (100.00%)         0s
...............

===== 28 passed in 0.98s =====

I am not that familiar with console.py, so I opted not to try to gut the _IPython class.

xref ipython/ipython#9504

Fix #11005

TODO

Finalize scope of refactoring.
Add change log.
Open issue about pyreadline after merge.

saimn · 2020-10-26T20:19:18Z

There are other uses of IOStream in this file, and I guess most of _IPython could be removed (not OutStream though).

pllim · 2020-10-26T22:00:04Z

@saimn , I am not convinced that _IPython needs to be gutted right now. I don't feel like refactoring the whole console.py. 😬

As you can see, most of the class is called here or there, though whether the calls are still necessary or not is another story.

$ grep "_IPython" -rn .
./astropy/utils/console.py:41:class _IPython:
./astropy/utils/console.py:138:    if (_IPython.OutStream is None or
./astropy/utils/console.py:139:        (not isinstance(file, _IPython.OutStream) and
./astropy/utils/console.py:140:         not isinstance(file, _IPython.IOStream))):
./astropy/utils/console.py:243:    if sys.platform == 'win32' and _IPython.OutStream is None:
./astropy/utils/console.py:275:    if (_IPython.IOStream is not None and
./astropy/utils/console.py:276:        isinstance(fileobj, _IPython.IOStream)):
./astropy/utils/console.py:666:                _IPython.get_ipython()

saimn · 2020-10-26T22:09:16Z

Well, the other uses of IOStream should also be removed ?

embray · 2020-10-27T12:40:58Z

You can probably also get rid of these lines: https://github.com/astropy/astropy/pull/10942/files#diff-582a94e0a0f7293634d5bb6531b7bdf8a847ecd2a380ec691a65bac8beaff546R137

pllim · 2020-10-27T17:41:18Z

@embray , can you please clarify #10942 (comment) ? That link points back to the diff that I made, so not sure if I understand.

saimn · 2020-10-28T01:50:16Z

@pllim - I'm not sure to understand everything in this module, but I would say that we should keep OutStream, and remove IOStream. So probably a bit too far ;).

pllim · 2020-10-30T17:22:36Z

Tests passed. Not sure about the coverage. Someone familiar with IPython stream should review. Thanks!

embray · 2020-11-08T16:39:35Z

@pllim Sorry for the late reply. I don't know why my link didn't work (it was some code that was not in the default context of one of your diffs, but GH didn't expand the context when clicking it. Nevertheless, it looks like in your followup commit you did address exactly the code I was referring to.

embray

Makes sense to me. I tested this (both running the tests, and some manual testing) on IPython down to 4.2.1. Since it works that far back let's keep the pyreadline stuff for now and just leave the note that it can be removed one day in the future. In the meantime supporting old IPython versions is not a bad idea.

embray · 2020-11-08T16:43:53Z

astropy/utils/console.py

    # - File name is 'stdout'; or
    # - File wraps a Console
    if getattr(file, 'name', None) == 'stdout':
        return True

    if hasattr(file, 'stream'):
+        # FIXME: pyreadline has no had new release since 2015, drop it?


Do we have a published minimum IPython version supported? IPython 5.x dropped dependency on pyreadline, over 4 years ago.

Sounds good!

pllim · 2020-11-10T19:13:08Z

@embray , I went ahead and defined minversion for ipython here. Does this look good now or did I miss something?

pllim · 2020-11-10T19:49:05Z

Hmm... Looking at https://travis-ci.com/github/astropy/astropy/jobs/432592934 using ipython==4.2.1 and traitlets==5.0.5, we either have to bump ipython or also pin traitlets. What do you think, @embray ?

==================================== ERRORS ====================================
�[31m�[1m_ ERROR collecting .tox/py37-test-oldestdeps/lib/python3.7/site-packages/astropy/table/pandas.py _�[0m
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/astropy/table/pandas.py�[0m:29: in <module>
    display.display(dhtml)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/IPython/core/display.py�[0m:152: in display
    format = InteractiveShell.instance().display_formatter.format
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/config/configurable.py�[0m:537: in instance
    inst = cls(*args, **kwargs)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/IPython/core/interactiveshell.py�[0m:441: in __init__
    self.init_ipython_dir(ipython_dir)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/IPython/core/interactiveshell.py�[0m:552: in init_ipython_dir
    self.ipython_dir = get_ipython_dir()
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:604: in __set__
    self.set(obj, value)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:593: in set
    obj._notify_trait(self.name, old_value, new_value)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:1222: in _notify_trait
    type='change',
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:1227: in notify_change
    return self._notify_observers(change)
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:1248: in _notify_observers
    "use @observe and @unobserve instead.")
�[1m�[31m../../.tox/py37-test-oldestdeps/lib/python3.7/site-packages/traitlets/traitlets.py�[0m:163: in _deprecated_method
    warn_explicit(warn_msg, DeprecationWarning, fname, lineno)
�[1m�[31mE   DeprecationWarning: InteractiveShell._ipython_dir_changed is deprecated in traitlets 4.1: use @observe and @unobserve instead.�[0m
=========================== short test summary info ============================

embray · 2020-11-12T17:12:33Z

That's annoying indeed. I had a look at the setup.py for IPython 4.2 and in fact it does not pin a version of traitlets at all.

So we could either pin it to traitlets<4.1 (just for the sake of the tests, in tox.ini).

Or do something annoying like:

diff --git a/astropy/table/pandas.py b/astropy/table/pandas.py
index 6c0004e7b..85a1f48b2 100644
--- a/astropy/table/pandas.py
+++ b/astropy/table/pandas.py
@@ -1,5 +1,7 @@
 # -*- coding: utf-8 -*-
 
+import warnings
+
 ascii_coded = ('Ò♙♙♙♙♙♙♙♙♌♐♐♌♙♙♙♙♙♙♌♌♙♙Ò♙♙♙♙♙♙♙♘♐♐♐♈♙♙♙♙♙♌♐♐♐♔Ò♙♙♌♈♙♙♌♐♈♈♙♙♙♙♙♙♙♙♈♐♐♙Ò♙♐♙♙♙♐♐♙♙♙'
                '♙♙♙♙♙♙♙♙♙♙♙♙Ò♐♔♙♙♘♐♐♙♙♌♐♐♔♙♙♌♌♌♙♙♙♌Ò♐♐♙♙♘♐♐♌♙♈♐♈♙♙♙♈♐♐♙♙♘♔Ò♐♐♌♙♘♐♐♐♌♌♙♙♌♌♌♙♈♈♙♌♐'
                '♐Ò♘♐♐♐♌♐♐♐♐♐♐♌♙♈♙♌♐♐♐♐♐♔Ò♘♐♐♐♐♐♐♐♐♐♐♐♐♈♈♐♐♐♐♐♐♙Ò♙♘♐♐♐♐♈♐♐♐♐♐♐♙♙♐♐♐♐♐♙♙Ò♙♙♙♈♈♈♙♙♐'
@@ -26,7 +28,14 @@ try:
                 return self.backup_text
 
     dhtml = HTMLWithBackup(html, ascii_uncoded)
-    display.display(dhtml)
+    with warnings.catch_warnings():
+        # ignore deprecation warning from newer traitlets versions
+        # used with older IPython versions
+        # https://github.com/astropy/astropy/pull/10942#issuecomment-724927877
+        warnings.filterwarnings('ignore', '.*traitlets 4.1.*',
+                DeprecationWarning)
+
+        display.display(dhtml)
 except ImportError:
     print(ascii_uncoded)
 except (UnicodeEncodeError, SyntaxError):

pllim · 2020-11-12T17:23:05Z

I think IPython not pinning their stuff isn't our problem. I'll try pinning traitlets in tox.ini as you suggested once we get the CI running again over at #10388

DOC: Add change logs.

pllim · 2020-11-12T20:43:11Z

@embray , you can ignore the remote data job. What do you think of the PR now?

embray · 2020-11-15T22:13:33Z

I think IPython not pinning their stuff isn't our problem

My thoughts exactly.

pllim requested review from embray and saimn October 26, 2020 19:56

pllim added utils Refactoring no-changelog-entry-needed labels Oct 26, 2020

pllim added this to the v4.3 milestone Oct 26, 2020

pllim force-pushed the console-ipython-stream branch from 68cc237 to 52036da Compare October 27, 2020 18:07

This comment has been minimized.

Sign in to view

pllim removed the no-changelog-entry-needed label Oct 27, 2020

pllim force-pushed the console-ipython-stream branch from 52036da to a67781d Compare October 28, 2020 14:14

embray approved these changes Nov 8, 2020

View reviewed changes

embray mentioned this pull request Nov 8, 2020

Document a minimum supported IPython version? #11005

Closed

pllim force-pushed the console-ipython-stream branch from a67781d to 46fdeab Compare November 10, 2020 19:01

pllim added the Ready-for-final-review label Nov 10, 2020

pllim added 3 commits November 12, 2020 15:15

MNT: Do not use IPython stream

5ad0a5c

Remove only _IPython IOStream

e1698f1

MNT: Set ipython minversion to 4.2

aa44b80

DOC: Add change logs.

pllim force-pushed the console-ipython-stream branch from d28aaee to aa44b80 Compare November 12, 2020 20:20

embray merged commit 0a1cd55 into astropy:master Nov 15, 2020

pllim deleted the console-ipython-stream branch November 16, 2020 15:16

pllim mentioned this pull request Feb 8, 2024

Remove obsolete code from utils/console.py #16012

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MNT: Do not use IPython stream in console #10942

MNT: Do not use IPython stream in console #10942

pllim commented Oct 26, 2020 •

edited

Loading

saimn commented Oct 26, 2020 •

edited

Loading

pllim commented Oct 26, 2020

saimn commented Oct 26, 2020

embray commented Oct 27, 2020

pllim commented Oct 27, 2020

This comment has been minimized.

saimn commented Oct 28, 2020

pllim commented Oct 30, 2020

embray commented Nov 8, 2020

embray left a comment

embray Nov 8, 2020

pllim Nov 10, 2020

pllim commented Nov 10, 2020

pllim commented Nov 10, 2020

embray commented Nov 12, 2020

pllim commented Nov 12, 2020 •

edited

Loading

pllim commented Nov 12, 2020

embray commented Nov 15, 2020

MNT: Do not use IPython stream in console #10942

MNT: Do not use IPython stream in console #10942

Conversation

pllim commented Oct 26, 2020 • edited Loading

Description

saimn commented Oct 26, 2020 • edited Loading

pllim commented Oct 26, 2020

saimn commented Oct 26, 2020

embray commented Oct 27, 2020

pllim commented Oct 27, 2020

This comment has been minimized.

saimn commented Oct 28, 2020

pllim commented Oct 30, 2020

embray commented Nov 8, 2020

embray left a comment

Choose a reason for hiding this comment

embray Nov 8, 2020

Choose a reason for hiding this comment

pllim Nov 10, 2020

Choose a reason for hiding this comment

pllim commented Nov 10, 2020

pllim commented Nov 10, 2020

embray commented Nov 12, 2020

pllim commented Nov 12, 2020 • edited Loading

pllim commented Nov 12, 2020

embray commented Nov 15, 2020

pllim commented Oct 26, 2020 •

edited

Loading

saimn commented Oct 26, 2020 •

edited

Loading

pllim commented Nov 12, 2020 •

edited

Loading