Skip to content

Conversation

@pllim
Copy link
Member

@pllim pllim commented Jun 7, 2021

Description

This pull request is to fix #11804 .

A direct follow-up of #11796. Also see numpy/numpy#19153.

p.s. Ignore the linkcheck failure.

@pllim pllim added Affects-dev PRs and issues that do not impact an existing Astropy release no-changelog-entry-needed Extra CI Run cron CI as part of PR labels Jun 7, 2021
@pllim pllim added this to the v5.0 milestone Jun 7, 2021
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@mhvk mhvk added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Jun 7, 2021
@pllim
Copy link
Member Author

pllim commented Jun 7, 2021

Thanks for the approval! We'll find out in a few hours. 😸

@mhvk
Copy link
Contributor

mhvk commented Jun 7, 2021

Darn, it the xfail didn't work - it ran the test and complained it failed:

self = <astropy.units.tests.test_quantity_array_methods.TestQuantityReshapeFuncs object at 0x415d6d9128>

    @pytest.mark.xfail(IS_S390X and NUMPY_LT_1_22, reason="Numpy GitHub Issue 19153")
    def test_flat_attributes(self):
        """While ``flat`` doesn't make a copy, it changes the shape."""
        q = np.arange(6.).reshape(3, 1, 2) * u.m
        qf = q.flat
        # see TestQuantityArrayCopy.test_flat for tests of iteration
        # and slicing and setting. Here we test the properties and methods to
        # match `numpy.ndarray.flatiter`
        assert qf.base is q
        # testing the indices -- flat and full -- into the array
        assert qf.coords == (0, 0, 0)  # to start
        assert qf.index == 0
        # now consume the iterator
        endindices = [(qf.index, qf.coords) for x in qf][-2]  # next() oversteps
>       assert endindices[0] == 5
E       assert 0 == 5

astropy/units/tests/test_quantity_array_methods.py:143: AssertionError

from astropy import units as u
from astropy.utils.compat import NUMPY_LT_1_22

IS_S390X = os.environ.get('ARCH_ON_CI', 'normal') == 's390x'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be this line...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the xfail not working? 🤯

ARCH_ON_CI: s390x
Numpy: 1.20.3

@mhvk
Copy link
Contributor

mhvk commented Jun 7, 2021

If we're sure it fails because it is big-endian, one could do

xfail(sys.byteorder == 'big' and NUMPY_LT_1_22)

@pllim
Copy link
Member Author

pllim commented Jun 7, 2021

@mhvk , good idea. Let's try that but have to wait a few more hours. 😿

@pllim
Copy link
Member Author

pllim commented Jun 7, 2021

@mhvk , finally green! Should I squash and [ci skip] or should we just merge?

@mhvk
Copy link
Contributor

mhvk commented Jun 7, 2021

Let's go for it! Thanks again!

@mhvk mhvk merged commit 5e65c36 into astropy:main Jun 7, 2021
@pllim pllim deleted the tst-flatiter-s390x branch June 7, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release Extra CI Run cron CI as part of PR no-changelog-entry-needed units utils zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TST: TestQuantityReshapeFuncs.test_flat_attributes failed with AssertionError on s390x (big-endian)

2 participants