Visidata: [Bug] [v2.-4dev] dup-rows/-deep does not copy rows

Created on 25 Mar 2020  ·  3Comments  ·  Source: saulpw/visidata

I found that if I try and do a dup-rows-deep with a DirSheet I get the following error with no rows in the sheet (columns are correct and as expected).

This only appears to happen with a DirSheet, I just tried with a TextSheet which works as expected.

Traceback (most recent call last):
  File "/path/vd_plugins/radare2/r2/lib/python3.7/site-packages/visidata-2._4dev-py3.7.egg/visidata/threads.py", line 201, in _toplevelTryFunc
    t.status = func(*args, **kwargs)
  File "/path/vd_plugins/radare2/r2/lib/python3.7/site-packages/visidata-2._4dev-py3.7.egg/visidata/threads.py", line 80, in _async_deepcopy
    newlist.append(deepcopy(r))
  File "/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/copy.py", line 281, in _reconstruct
    if hasattr(y, '__setstate__'):
  File "/path/vd_plugins/radare2/r2/lib/python3.7/site-packages/visidata-2._4dev-py3.7.egg/visidata/path.py", line 77, in __getattr__
    r = getattr(self._path, k)
  File "/path/vd_plugins/radare2/r2/lib/python3.7/site-packages/visidata-2._4dev-py3.7.egg/visidata/path.py", line 77, in __getattr__
    r = getattr(self._path, k)
  File "/path/vd_plugins/radare2/r2/lib/python3.7/site-packages/visidata-2._4dev-py3.7.egg/visidata/path.py", line 77, in __getattr__
    r = getattr(self._path, k)
  [Previous line repeated 492 more times]
RecursionError: maximum recursion depth exceeded

Replicate:

  1. Use v2.-4dev branch
  2. Open a DirSheet with vd .
  3. Do a deep copy of the sheet with dup-rows-deep

Result:

A sheet with no rows, columns as expected for a DirSheet, and the error message above.

Expected:

A deep copy of the original DirSheet with all rows (including those selected)

cmdlog:

sheet   col row longname    input   keystrokes  comment
    SqliteSheet header  set-option  0
    UsvSheet    delimiter   set-option  ␞
    UsvSheet    row_delimiter   set-option  ␟
    override    disp_date_fmt   set-option  %Y-%m-%d %H:%M:%S
            open-file   .   o
_files          dup-rows-deep       gz" open duplicate sheet with deepcopy of all rows

Also, am I correct in understanding that if I make a deep copy, modifications I make to that copy should propagate to the original sheet? And this should include selecting/deselecting rows?

bug

Most helpful comment

Fixed!

Here is the really complicated explanation for why this awesome bug was happening:

  • previously assumed that every time vdPath does not have the
    attribute, _path (pathlib.Path) would
  • when _path lacked the attribute, AttributeError was not being raised
  • instead VisiData would look for that attribute in an additional layer
    of _path (since looking for it would invoke __getattr__ again), and look for _path forever until Python put it out of its
    misery
  • this commit first checks for the existence of _path

Fixes #489, which was triggering this recursion since deepcopy() was
checking for the __setstate__ attribute, which pathlib.Path objects do
not have implemented.

Thank you so much for this bug report @geekscrapy!

All 3 comments

Hi @geekscrapy, thanks for this bug report, I'll take a look when I get a chance.

Also, am I correct in understanding that if I make a deep copy, modifications I make to that copy should propagate to the original sheet? And this should include selecting/deselecting rows?

No; a deep copy explicitly copies all the values, so that any modifications are not reflected on the source sheet. For modifications to propagate, use " which pulls selected rows only into a new sheet, or g" which pulls all rows into a copy of the sheet, keeping the same set of rows selected. Note that in both cases, only edits will be propagated; selecting rows is not. The propagation of row selection only happens with Frequency Tables and Graphs, and was implemented specifically for those sheets.

Thanks. My apologies, in that last statement I got mixed up and meant " or g" copy. Tested with other sheets and see how it works now.

I presume I might be able to take the logic from freq table and add that to my own sheet (as it's a needed feature for me).

Thanks again!

Fixed!

Here is the really complicated explanation for why this awesome bug was happening:

  • previously assumed that every time vdPath does not have the
    attribute, _path (pathlib.Path) would
  • when _path lacked the attribute, AttributeError was not being raised
  • instead VisiData would look for that attribute in an additional layer
    of _path (since looking for it would invoke __getattr__ again), and look for _path forever until Python put it out of its
    misery
  • this commit first checks for the existence of _path

Fixes #489, which was triggering this recursion since deepcopy() was
checking for the __setstate__ attribute, which pathlib.Path objects do
not have implemented.

Thank you so much for this bug report @geekscrapy!

Was this page helpful?
0 / 5 - 0 ratings