TST: pyyaml 5.1 broke several tests

[pllim]

pyyaml 5.1 was released yesterday (Mar 13, 2019) -- https://pypi.org/project/PyYAML/#history

Since then, some tests are broken. They affect tests in both PR and cron levels, wherever we pull in pyyaml from PyPI via pip. Example log: https://travis-ci.org/astropy/astropy/jobs/506241000

______________________________ test_write_simple _______________________________
    @pytest.mark.skipif('not HAS_YAML')
    def test_write_simple():
        """
        Write a simple table with common types.  This shows the compact version
        of serialization with one line per column.
        """
        t = simple_table()
    
        out = StringIO()
        t.write(out, format='ascii.ecsv')
>       assert out.getvalue().splitlines() == SIMPLE_LINES
E       AssertionError: assert ['# %ECSV 0.9...name: b', ...] == ['# %ECSV 0.9'...string}', ...]
E         At index 3 diff: '# - name: a' != '# - {name: a, datatype: int64}'
E         Left contains more items, first extra item: '1 1.0 c'
E         Use -v to get the full diff
astropy/io/ascii/tests/test_ecsv.py:80: AssertionError
_______________________________ test_write_full ________________________________
    @pytest.mark.skipif('not HAS_YAML')
    def test_write_full():
        """
        Write a full-featured table with common types and explicitly checkout output
        """
        t = T_DTYPES['bool', 'int64', 'float64', 'str']
        lines = ['# %ECSV 0.9',
                 '# ---',
                 '# datatype:',
                 '# - name: bool',
                 '#   unit: m / s',
                 '#   datatype: bool',
                 '#   description: descr_bool',
                 '#   meta: {meta bool: 1}',
                 '# - name: int64',
                 '#   unit: m / s',
                 '#   datatype: int64',
                 '#   description: descr_int64',
                 '#   meta: {meta int64: 1}',
                 '# - name: float64',
                 '#   unit: m / s',
                 '#   datatype: float64',
                 '#   description: descr_float64',
                 '#   meta: {meta float64: 1}',
                 '# - name: str',
                 '#   unit: m / s',
                 '#   datatype: string',
                 '#   description: descr_str',
                 '#   meta: {meta str: 1}',
                 '# meta: !!omap',
                 '# - comments: [comment1, comment2]',
                 '# schema: astropy-2.0',
                 'bool int64 float64 str',
                 'False 0 0.0 "ab 0"',
                 'True 1 1.0 "ab, 1"',
                 'False 2 2.0 ab2']
    
        out = StringIO()
        t.write(out, format='ascii.ecsv')
>       assert out.getvalue().splitlines() == lines
E       AssertionError: assert ['# %ECSV 0.9...e: bool', ...] == ['# %ECSV 0.9'...e: bool', ...]
E         At index 7 diff: '#   meta:' != '#   meta: {meta bool: 1}'
E         Left contains more items, first extra item: '#   - comment2'
E         Use -v to get the full diff
astropy/io/ascii/tests/test_ecsv.py:122: AssertionError
________________________ [doctest] astropy.io.misc.yaml ________________________
031   >>> from astropy.io.misc import yaml
032   >>> import astropy.units as u
033   >>> from astropy.time import Time
034   >>> from astropy.coordinates import EarthLocation
035 
036   >>> t = Time(2457389.0, format='mjd',
037   ...          location=EarthLocation(1000, 2000, 3000, unit=u.km))
038   >>> td = yaml.dump(t)
039 
040   >>> print(td)
Differences (unified diff with -expected +actual):
    @@ -7,5 +7,6 @@
       ellipsoid: WGS84
       x: !astropy.units.Quantity
    -    unit: &id001 !astropy.units.Unit {unit: km}
    +    unit: &id001 !astropy.units.Unit
    +      unit: km
         value: 1000.0
       y: !astropy.units.Quantity
    @@ -18,2 +19,3 @@
     precision: 3
     scale: utc
    +<BLANKLINE>
/tmp/astropy-test-gwug00ji/lib/python3.6/site-packages/astropy/io/misc/yaml.py:40: DocTestFailure
___________________________ test_fail_meta_serialize ___________________________
tmpdir = local('/tmp/pytest-of-travis/pytest-0/test_fail_meta_serialize0')
    @pytest.mark.skipif('not HAS_H5PY or not HAS_YAML')
    def test_fail_meta_serialize(tmpdir):
    
        test_file = str(tmpdir.join('test.hdf5'))
    
        t1 = Table()
        t1.add_column(Column(name='a', data=[1, 2, 3]))
        t1.meta['f'] = str
    
        with pytest.raises(Exception) as err:
            t1.write(test_file, path='the_table', serialize_meta=True)
>       assert "cannot represent an object: <class 'str'>" in str(err)
E       assert "cannot represent an object: <class 'str'>" in "/home/travis/miniconda/envs/test/lib/python3.6/site-packages/yaml/representer.py:231: yaml.representer.RepresenterError: ('cannot represent an object', <class 'str'>)"
E        +  where "/home/travis/miniconda/envs/test/lib/python3.6/site-packages/yaml/representer.py:231: yaml.representer.RepresenterError: ('cannot represent an object', <class 'str'>)" = str(<ExceptionInfo RepresenterError tblen=21>)
astropy/io/misc/tests/test_hdf5.py:541: AssertionError
====== 4 failed, 12402 passed, 299 skipped, 105 xfailed in 236.55 seconds ======

@drdavella and @Cadair pointed to asdf-format/asdf#659 and yaml/pyyaml#256

[bsipocz]

Shall we just then pin to the old version as a workaround to at least keep the CI passing for PRs?

[pllim]

Re: pinning -- See #8499

[drdavella]

We had the same problem in ASDF (see asdf-format/asdf#659). This appears to be due to a backwards-incompatible change in the default setting for flow style in 5.1: yaml/pyyaml#256.

EDIT Sorry for the duplicate comment. I missed what @pllim said at the bottom of the comment above.

[pllim]

cross-post: #8499 (comment)

tl;dr -- Need advice from @taldcroft on the best way forward.

[taldcroft]

😢 Booh on PyYAML. I'll put this on the list of things to look at before the next release.

The important question is about the round-tripping tests still passing. I.e. we have tests like this one that are testing the literal serialization output, and it's (more-or-less) OK if those are failing with PyYAML 5.1. What's not OK is if the functional round-trip tests are failing. But from what I see in the output above, the only tests that are failing are just related to the formatting of the YAML.

So the short-term bandaid of pinning PyYAML to an earlier version seems OK. If people using astropy upgrade PyYAML there shouldn't be a problem.