gnuradio/pybombs

Pybombs breaks with ruamel.yaml 0.15.56

Closed this issue · 5 comments

Tried to install the rfnoc dev environment under Ubuntu 18.04 with the following commands

$ pybombs recipes add gr-recipes git+https://github.com/gnuradio/gr-recipes.git
$ pybombs recipes add ettus git+https://github.com/EttusResearch/ettus-pybombs.git
$ pybombs prefix init ~/rfnoc_test -R rfnoc -a rfnoc_test

The last command gave me this error:

pybombs prefix init ~/rfnoc_test -R rfnoc -a rfnoc_test
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS - INFO - PyBOMBS Version 2.3.3a0
PyBOMBS.prefix - INFO - Creating directory `/home/margot/rfnoc_test'
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS.ConfigManager - INFO - Creating new config file /home/margot/rfnoc_test/.pybombs/config.yml
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS.prefix - INFO - Installing default packages for prefix...
PyBOMBS.prefix - INFO - 
  - <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0>
  - <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c4500>
PyBOMBS.Packager.apt - INFO - Install python-apt to speed up apt processing.
PyBOMBS.install_manager - INFO - Phase 1: Creating install tree and installing binary packages:
PyBOMBS.get_recipe - ERROR - Error fetching recipe `<ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0>':
Package <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0> has no recipe file!

Since I just had successfully installed this under Ubuntu 16.04 I figured this must be connected to a difference in the ruamel.yaml versions between the installations. My current machine has ruamel.yaml version 0.15.56 installed and the working machine has version 0.15.44.

Therefore, I tried downgrading ruamel.yaml to version 0.15.44, which resolved the problem. However I dont think 0.15.56 is the particular issue, since the changelog for version 0.15.55 states:

unmade CommentedSeq a subclass of list. It is now indirectly a subclass of the standard collections.abc.MutableSequence (without .abc if you are still on Python2.7). If you do isinstance(yaml.load('[1, 2]'), list)) anywhere in your code replace list with MutableSequence. Directly, CommentedSeq is a subclass of the abstract baseclass ruamel.yaml.compat.MutableScliceableSequence, with the result that (extended) slicing is supported on CommentedSeq. (reported by Stuart Berg)

I think this might be the cause for the underlying issue but I had no time to verify that.

Same here.
@Bustel how did you downgrade ruamel.yaml?

@ido1990 pip lets you install specific versions. The following did the trick for me:

pip uninstall ruamel.yaml
pip install ruamel.yaml==0.15.44
AvdN commented

Is there an easy way for me to test whether my changes broke things and I need to fix ruamel.yaml.

As indicated: CommentedSeq used to be a subclass of list. It now has a list attribute and is dependent on collections.abc.MutableSequence. That latter is an abstract class and I implemented all required classes (like getitem, setitem), but there are some method like .copy() and .sort(), that I did get automagically from list, but not from MutableSequence. I implemented those two, but there might be more missing, which should be very simple once I know what they are. From the error here I have no clue though.

If so you should probably be able to install ruamel.yaml==0.15.54 without hitting this issue.

This might however be related to the earler change in 0.15.52 from CommentedMap being subclass of dict to subclass of collections.abc.MutableMapping

AvdN commented

Based on myinitial investigation I suspects that this might be related to dict_merge` in pybombs/utils/utils.py``:

def dict_merge(a, b):
    """
    Recursively merge b into a. b[k] will overwrite a[k] if it exists.
    """
    if not isinstance(b, dict):
        return b
    result = deepcopy(a)
    for k, v in iteritems(b):
        if k in result and isinstance(result[k], dict):
            result[k] = dict_merge(result[k], v)
        else:
            result[k] = deepcopy(v)
    return result

That isinstance(b, dict looks fishy, and would better be isinstance(b, MutableMapping) (every dict is a MutableMapping, but not every MutableMapping is a dict). You might even use Mapping.

Take note that (Mutable)Mapping comes from collections.abc on Py3 and ``collections` on Py2

Merged @AvdN's fix.