ls: Use pyout #399

kyleam · 2019-04-01T14:41:47Z

Closes #318.

Ls.__call__() returns a dictionary with results to ease testing. ls.py will start using pyout in the next commit. Once that happens, it makes sense to drop the custom result dictionary because the Tabular object already contains the same information and provides access to the values _after_ the asynchronous functions. But before we do that, convert the result dictionary to a structure that more closely matches the Tabular getitem interface so that we can switch to pyout *without* modifying the actual tests.

kyleam · 2019-04-01T14:56:15Z

The length bit of test_no_heavy_imports fails. I'll take a look at what's being pulled in.

codecov · 2019-04-01T15:10:45Z

Codecov Report

Merging #399 into master will decrease coverage by 0.01%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master     #399      +/-   ##
==========================================
- Coverage   89.82%   89.81%   -0.02%     
==========================================
  Files         148      148              
  Lines       11658    11676      +18     
==========================================
+ Hits        10472    10487      +15     
- Misses       1186     1189       +3

Impacted Files	Coverage Δ
reproman/interface/tests/test_ls.py	`100% <100%> (ø)`	⬆️
reproman/interface/ls.py	`93.1% <89.18%> (-0.52%)`	⬇️
...eproman/interface/tests/test_backend_parameters.py	`95.65% <0%> (-4.35%)`	⬇️
reproman/tests/skip.py	`95% <0%> (-2.5%)`	⬇️
reproman/resource/docker_container.py	`95.3% <0%> (+0.63%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c199c04...bdfbc6d. Read the comment docs.

codecov · 2019-04-01T15:10:46Z

Codecov Report

Merging #399 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #399      +/-   ##
==========================================
+ Coverage   89.82%   89.87%   +0.04%     
==========================================
  Files         148      148              
  Lines       11658    11684      +26     
==========================================
+ Hits        10472    10501      +29     
+ Misses       1186     1183       -3

Impacted Files	Coverage Δ
reproman/interface/tests/test_ls.py	`100% <100%> (ø)`	⬆️
reproman/interface/ls.py	`100% <100%> (+6.38%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c199c04...afd47ef. Read the comment docs.

kyleam · 2019-04-01T15:13:27Z

The length bit of test_no_heavy_imports fails. I'll take a look at what's being pulled in.

It was mostly what you'd expect by importing pyout at the top-level:

diff of modules

--- modules-master	2019-04-01 10:58:44.273588743 -0400
+++ modules-pr	2019-04-01 10:58:22.400929834 -0400
@@ -1,4 +1,5 @@
 {'__future__',
+ '__mp_main__',
  '_ast',
  '_bisect',
  '_bz2',
@@ -6,6 +7,7 @@
  '_compat_pickle',
  '_compression',
  '_ctypes',
+ '_curses',
  '_datetime',
  '_functools',
  '_hashlib',
@@ -24,6 +26,7 @@
  '_virtualenv_distutils',
  'appdirs',
  'argparse',
+ 'array',
  'ast',
  'atexit',
  'attr',
@@ -38,6 +41,7 @@
  'base64',
  'binascii',
  'bisect',
+ 'blessings',
  'bz2',
  'calendar',
  'collections',
@@ -49,6 +53,7 @@
  'ctypes',
  'ctypes._endian',
  'ctypes.util',
+ 'curses',
  'datetime',
  'dis',
  'distutils',
@@ -71,6 +76,7 @@
  'email.quoprimime',
  'email.utils',
  'enum',
+ 'fcntl',
  'fnmatch',
  'functools',
  'gc',
@@ -101,11 +107,24 @@
  'logging.handlers',
  'lzma',
  'math',
+ 'multiprocessing',
+ 'multiprocessing.context',
+ 'multiprocessing.dummy',
+ 'multiprocessing.dummy.connection',
+ 'multiprocessing.process',
  'opcode',
  'operator',
  'pickle',
  'platform',
  'pwd',
+ 'pyout',
+ 'pyout.common',
+ 'pyout.elements',
+ 'pyout.field',
+ 'pyout.interface',
+ 'pyout.summary',
+ 'pyout.tabular',
+ 'pyout.truncate',
  'pytz',
  'pytz.exceptions',
  'pytz.lazy',
@@ -177,6 +196,7 @@
  'shlex',
  'shutil',
  'signal',
+ 'six',
  'socket',
  'sre_compile',
  'sre_constants',

I've moved the pyout import to Ls.__call__.

range-diff

1:  0d092d042 = 1:  0d092d042 setup.py: Add pyout
2:  1cfe18525 = 2:  1cfe18525 TST: ls: Adjust return structure to mirror pyout
3:  ed4b31442 ! 3:  bdfbc6d02 ENH: ls: Use pyout to render table
    @@ -25,8 +25,6 @@

     -from collections import OrderedDict
     +from functools import partial
    -+
    -+from pyout import Tabular

      from .base import Interface
      # import reproman.interface.base  # Needed for test patching
    @@ -38,7 +36,8 @@
     -        template = '{:<20} {:<20} {:<%(id_length)s} {!s:<10}' % locals()
     -        ui.message(template.format('RESOURCE NAME', 'TYPE', 'ID', 'STATUS'))
     -        ui.message(template.format('-------------', '----', '--', '------'))
    --
    ++        from pyout import Tabular
    +
     -        results = OrderedDict()
              manager = get_manager()
              if not resrefs:
    @@ -184,8 +183,7 @@
                                            return_value=resource_manager))
     -            stack.enter_context(patch("reproman.interface.ls.ui._ui.out",
     -                                      stream))
    -+            stack.enter_context(patch("reproman.interface.ls.Tabular",
    -+                                      TestTabular))
    ++            stack.enter_context(patch("pyout.Tabular", TestTabular))
                  return ls(*args, **kwargs), stream.getvalue()
          return fn

reproman/interface/ls.py

The main benefit of using pyout here is that we can query statuses asynchronously, but it also allows us to style the output in more complex ways (automatic truncation, coloring by regexp, ...) with less custom code than we'd need otherwise. This switch to pyout should not change how we interpret or update the resource statuses, but the code for interacting with the resources is a little more complicated because, when --refresh is given, we access the resources in functions that are called asynchronously. And to update the resource statuses, we need to look at the values after the asynchronous calls have returned. Closes ReproNim#318.

kyleam · 2019-04-01T21:35:35Z

Marking this as WIP because I consider the issue @yarikoptic raises in #400 to be a blocker.

kyleam added 2 commits April 1, 2019 10:37

setup.py: Add pyout

0d092d0

kyleam force-pushed the ls-pyout branch from 1fd60a4 to ed4b314 Compare April 1, 2019 14:45

kyleam added the WiP label Apr 1, 2019

kyleam force-pushed the ls-pyout branch from ed4b314 to bdfbc6d Compare April 1, 2019 15:10

kyleam removed the WiP label Apr 1, 2019

kyleam commented Apr 1, 2019

View reviewed changes

reproman/interface/ls.py Show resolved Hide resolved

kyleam added the WiP label Apr 1, 2019

kyleam added 2 commits April 1, 2019 12:34

TST: ls: Check handling of non-existent resource

8ed7a7a

kyleam force-pushed the ls-pyout branch from bdfbc6d to afd47ef Compare April 1, 2019 16:36

kyleam removed the WiP label Apr 1, 2019

yarikoptic mentioned this pull request Apr 1, 2019

pyout: "Interference" between pyout output and interactive prompt #400

Open

kyleam added the WiP label Apr 1, 2019

yarikoptic marked this pull request as draft February 15, 2023 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ls: Use pyout #399

ls: Use pyout #399

kyleam commented Apr 1, 2019

kyleam commented Apr 1, 2019

codecov bot commented Apr 1, 2019

codecov bot commented Apr 1, 2019 •

edited

kyleam commented Apr 1, 2019

kyleam commented Apr 1, 2019

ls: Use pyout #399

Are you sure you want to change the base?

ls: Use pyout #399

Conversation

kyleam commented Apr 1, 2019

kyleam commented Apr 1, 2019

codecov bot commented Apr 1, 2019

Codecov Report

codecov bot commented Apr 1, 2019 • edited

Codecov Report

kyleam commented Apr 1, 2019

kyleam commented Apr 1, 2019

codecov bot commented Apr 1, 2019 •

edited