intake / intake-xarray

Intake plugin for xarray
https://intake-xarray.readthedocs.io/
BSD 2-Clause "Simplified" License
76 stars 36 forks source link

Bug involving persistence and (safe) YAML #44

Closed danielballan closed 5 years ago

danielballan commented 5 years ago

I get a test failure locally running intake-xarray on master and intake==0.5.1. It looks like the persistence is encoding a Python tuple as opposed to a generic sequence, and the "safe" YAML loader can't decode it.

________________________________________________________________________________________ test_persist ________________________________________________________________________________________

catalog1 = <Intake catalog: data>

    def test_persist(catalog1):
        from intake_xarray import ZarrSource
        source = catalog1['blank']
>       s2 = source.persist()

tests/test_catalog.py:27: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../intake/intake/source/base.py:283: in persist
    store.add(self._tok, out)
../intake/intake/container/persist.py:96: in add
    data = yaml.safe_load(f)
../../../venv/bnl/lib/python3.6/site-packages/yaml/__init__.py:162: in safe_load
    return load(stream, SafeLoader)
../../../venv/bnl/lib/python3.6/site-packages/yaml/__init__.py:114: in load
    return loader.get_single_data()
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:43: in get_single_data
    return self.construct_document(node)
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:52: in construct_document
    for dummy in generator:
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:404: in construct_yaml_map
    value = self.construct_mapping(node)
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:210: in construct_mapping
    return super().construct_mapping(node, deep=deep)
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:135: in construct_mapping
    value = self.construct_object(value_node, deep=deep)
../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:92: in construct_object
    data = constructor(self, node)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <yaml.loader.SafeLoader object at 0x7f77505fe470>
node = SequenceNode(tag='tag:yaml.org,2002:python/tuple', value=[ScalarNode(tag='tag:yaml.org,2002:str', value='lat'), Scalar...'level'), ScalarNode(tag='tag:yaml.org,2002:str', value='lon'), ScalarNode(tag='tag:yaml.org,2002:str', value='time')])

    def construct_undefined(self, node):
        raise ConstructorError(None, None,
                "could not determine a constructor for the tag %r" % node.tag,
>               node.start_mark)
E       yaml.constructor.ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/tuple'
E         in "/home/dallan/.intake/persisted/cat.yaml", line 115, column 17

../../../venv/bnl/lib/python3.6/site-packages/yaml/constructor.py:420: ConstructorError
martindurant commented 5 years ago

Now that is super-annoying. I'm surprised, though - tuples are pretty common, I would have thought we have them elsewhere. It should definitely be allowed by the safe loader (and sets too, although those are unlikely).

Hold on...

jbednar commented 5 years ago

We ran into the same problem in Datashader, and recoded our tuples as lists, but that's not always feasible...

martindurant commented 5 years ago

Turns out that YAML does support sets natively, though, so this is the only basic type missing.

danielballan commented 5 years ago

Thanks for the fast fix!