Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport PR #15473: Ensure tables with masked and unmasked columns roundtrip properly (v5.0.x) #15481

Merged
merged 1 commit into from
Oct 14, 2023

Conversation

mhvk
Copy link
Contributor

@mhvk mhvk commented Oct 13, 2023

Manual backport of #15473 onto v5.0.x

@mhvk mhvk added this to the v5.0.9 milestone Oct 13, 2023
@mhvk mhvk closed this Oct 13, 2023
@mhvk mhvk reopened this Oct 13, 2023
@pllim pllim added the skip-basebranch-check Skip base branch check for direct backports label Oct 13, 2023
@pllim pllim changed the title Backport PR #15473: Ensure tables with masked and unmasked columns roundtrip properly Backport PR #15473: Ensure tables with masked and unmasked columns roundtrip properly (v5.0.x) Oct 13, 2023
@pllim
Copy link
Member

pllim commented Oct 13, 2023

You need the skip basebranch check label. I will close/reopen again.

@pllim pllim closed this Oct 13, 2023
@pllim pllim reopened this Oct 13, 2023
@pllim pllim added the skip-changelog-checks Tells bot to skip changlog checks label Oct 13, 2023
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Thanks!

@pllim pllim enabled auto-merge October 13, 2023 19:35
@pllim
Copy link
Member

pllim commented Oct 13, 2023

Hmm @mhvk , does pyerfa not provide wheel for this?

  Collecting pyerfa>=2.0 (from astropy==5.0.9.dev29+g7efa256707)
    Downloading pyerfa-2.0.1.tar.gz (817 kB)
       ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 817.3/817.3 kB 78.1 MB/s eta 0:00:00
    Installing build dependencies: started
    Installing build dependencies: finished with status 'error'

@pllim pllim disabled auto-merge October 13, 2023 20:06
@mhvk
Copy link
Contributor Author

mhvk commented Oct 13, 2023

Hmm, that's weird, I thought that with our new scheme we have wheels that are good for all python/numpy versions... Maybe this takes some time? I restarted the CI run just in case. @astrofrog, do you know whether there is a delay in pyerfa wheel building, or whether they for some reason would not work with astropy 5.0?

@astrofrog
Copy link
Member

Which job is not using PyERFA wheels?

@pllim
Copy link
Member

pllim commented Oct 13, 2023

cp39-manylinux_x86_64 wheels

@pllim
Copy link
Member

pllim commented Oct 14, 2023

Maybe it is not compatible with your abi3 wheels (https://pypi.org/project/pyerfa/2.0.1/#files). This branch still uses oldest-supported-numpy and have this pin:

numpy>=1.18,<1.25

In the event it cannot find wheel, it tries to build pyerfa from source but now pyerfa needs numpy>=1.25,<2 to build (https://github.com/liberfa/pyerfa/blob/1903739c3e971938c4b4e0e4994d3e3f6039ba71/pyproject.toml#L6).

Does this mean we need to pin pyerfa<2.0.1 in this branch?

@mhvk
Copy link
Contributor Author

mhvk commented Oct 14, 2023

Maybe we should relax the numpy constraint to build pyerfa from source? Pyerfa itself does not require that at all. If it is there just for wheel-building, then perhaps the constraint should be moved to that part (somehow).

@mhvk
Copy link
Contributor Author

mhvk commented Oct 14, 2023

p.s. Given that 5.0 is almost at its end, pinning pyerfa may be reasonable too. But personally I prefer to remove the numpy constraint in pyerfa; it build perfectly fine for older numpy.

@pllim
Copy link
Member

pllim commented Oct 14, 2023

I don't think we can if I understood the conversation here correctly.

@pllim
Copy link
Member

pllim commented Oct 14, 2023

At any rate, I think this backport should go in. So merging. Thanks!

@pllim pllim merged commit 6288c85 into astropy:v5.0.x Oct 14, 2023
44 of 58 checks passed
@pllim pllim mentioned this pull request Oct 14, 2023
1 task
@mhvk mhvk deleted the auto-backport-of-pr-15473-on-v5.0.x branch October 14, 2023 15:45
@mhvk
Copy link
Contributor Author

mhvk commented Oct 14, 2023

See liberfa/pyerfa#119 for a place to discuss how to ensure we can at least build pyerfa from source with all supported numpy versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug io.fits Manual Backport skip-basebranch-check Skip base branch check for direct backports skip-changelog-checks Tells bot to skip changlog checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants