Skip to content

Fix CI in v4.0.x branch - #10810

Merged
astrofrog merged 10 commits into
astropy:v4.0.xfrom
astrofrog:fix-v4.0.x-ci
Oct 9, 2020
Merged

astrofrog merged 10 commits into
astropy:v4.0.xfrom
astrofrog:fix-v4.0.x-ci

Conversation

@astrofrog

Copy link
Copy Markdown
Member

Work in progress!

@astrofrog astrofrog added this to the v4.0.2 milestone Oct 7, 2020
@pllim
pllim marked this pull request as draft October 7, 2020 13:41
@pllim

pllim commented Oct 7, 2020

Copy link
Copy Markdown
Member

I changed this into draft. When it is ready, you can change it back. 😉

@astrofrog

Copy link
Copy Markdown
Member Author

@taldcroft - I'm seeing failures in test_index_modes in the v4.0.x branch under certain conditions:

self = <astropy.table.tests.test_index.TestIndex object at 0xefb2806c>
main_col = [1, 2, 3, 4, 5]
table_types = <astropy.table.tests.conftest.table_types.<locals>.TableTypes object at 0xefb28d8c>
engine = <class 'astropy.table.sorted_array.SortedArray'>

    def test_index_modes(self, main_col, table_types, engine):
        self._setup(main_col, table_types)
        t = self.t
        t.add_index('a', engine=engine)
    
        # first, no special mode
        assert len(t[[1, 3]].indices) == 1
        assert len(t[::-1].indices) == 1
        assert len(self._table_type(t).indices) == 1
        assert np.all(t.indices[0].sorted_data() == [0, 1, 2, 3, 4])
        t2 = t.copy()
    
        # non-copy mode
        with t.index_mode('discard_on_copy'):
            assert len(t[[1, 3]].indices) == 0
            assert len(t[::-1].indices) == 0
            assert len(self._table_type(t).indices) == 0
            assert len(t2.copy().indices) == 1  # mode should only affect t
    
        # make sure non-copy mode is exited correctly
        assert len(t[[1, 3]].indices) == 1
    
        if not self.mutable:
            return
    
        # non-modify mode
        with t.index_mode('freeze'):
            assert np.all(t.indices[0].sorted_data() == [0, 1, 2, 3, 4])
            t['a'][0] = 6
            assert np.all(t.indices[0].sorted_data() == [0, 1, 2, 3, 4])
            t.add_row((1.5, '12', 2))
            assert np.all(t.indices[0].sorted_data() == [0, 1, 2, 3, 4])
            t.remove_rows([1, 3])
            assert np.all(t.indices[0].sorted_data() == [0, 1, 2, 3, 4])
            assert_col_equal(t['a'], [6, 3, 5, 2])
            # mode should only affect t
            assert np.all(t2.indices[0].sorted_data() == [0, 1, 2, 3, 4])
            t2['a'][0] = 6
            assert np.all(t2.indices[0].sorted_data() == [1, 2, 3, 4, 0])
    
        # make sure non-modify mode is exited correctly
        assert np.all(t.indices[0].sorted_data() == [3, 1, 2, 0])
    
        if isinstance(t['a'], BaseColumn):
>           assert len(t['a'][::-1].info.indices) == 0

astropy/table/tests/test_index.py:368: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
astropy/utils/data_info.py:214: in __get__
    return getattr(instance._parent, self.attr)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <[AttributeError('failed access "info" attribute on a temporary object.\n\nIt looks like you have done something like ...ermanent\nreference with ``c = col[3:5]`` followed by ``c.info``.',) raised in repr()] ColumnInfo object at 0xefcecdc4>

        @property
        def _parent(self):
            try:
                parent = self._parent_ref()
            except AttributeError:
                return None
    
            if parent is None:
                raise AttributeError("""\
    failed access "info" attribute on a temporary object.
    
    It looks like you have done something like ``col[3:5].info``, i.e.
    you accessed ``info`` from a temporary slice object ``col[3:5]`` that
    only exists momentarily.  This has failed because the reference to
    that temporary object is now lost.  Instead force a permanent
>   reference with ``c = col[3:5]`` followed by ``c.info``.""")
E   AttributeError: failed access "info" attribute on a temporary object.
E   
E   It looks like you have done something like ``col[3:5].info``, i.e.
E   you accessed ``info`` from a temporary slice object ``col[3:5]`` that
E   only exists momentarily.  This has failed because the reference to
E   that temporary object is now lost.  Instead force a permanent
E   reference with ``c = col[3:5]`` followed by ``c.info``.

astropy/utils/data_info.py:321: AttributeError

I'm trying to investigate this but I just thought I'd check with you in case this is something you know how to fix or have encountered in the past?

@astrofrog

Copy link
Copy Markdown
Member Author

How does this test normally not fail given that it does t['a'][::-1].info instead of the recommended syntax?

@astrofrog
astrofrog force-pushed the fix-v4.0.x-ci branch 7 times, most recently from 4cec586 to bf7522a Compare October 8, 2020 23:41
@pep8speaks

pep8speaks commented Oct 9, 2020 •

Copy link
Copy Markdown

Hello @astrofrog! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-10-09 09:49:56 UTC

@astrofrog
astrofrog marked this pull request as ready for review October 9, 2020 13:01
@pllim

pllim commented Oct 9, 2020

Copy link
Copy Markdown
Member

The remaining errors in numpy-dev/remote-data are being addressed in #10818 . You can either ignore DeprecationWarning: using a dtype with a subarray field is deprecated in setup.cfg to make it green, or proceed despite the red status.

@astrofrog

Copy link
Copy Markdown
Member Author

Since nothing here should be controversial, I'm just going to go ahead and merge this

@astrofrog
astrofrog merged commit c81beba into astropy:v4.0.x Oct 9, 2020
@astrofrog

Copy link
Copy Markdown
Member Author

@pllim oops I didn't see your message in time - I don't think that failure is critical to fix now, we can just backport from master once we have a fix for the warning there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants