Skip to content

Run limited API build on Mac in addition to Linux - #18250

Merged
astrofrog merged 4 commits into
astropy:mainfrom
astrofrog:limited-api-mac
Jun 10, 2025
Merged

astrofrog merged 4 commits into
astropy:mainfrom
astrofrog:limited-api-mac

Conversation

@astrofrog

@astrofrog astrofrog commented Jun 10, 2025 •

Copy link
Copy Markdown
Member

@neutrinoceros - the limited API build fails on Mac, presumably due to a different compiler. I'm first letting this fail to check, then I'll add an exception for stats.pyx.

@astrofrog
astrofrog marked this pull request as draft June 10, 2025 09:29
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@astrofrog

Copy link
Copy Markdown
Member Author

Just to confirm, the Mac build fails with:

 astropy/stats/src/fast_sigma_clip.c:109:29: error: call to undeclared function 'PyMem_RawMalloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    data_buffer = (double *)PyArray_malloc(n_i * sizeof(double));
                            ^
/private/var/folders/y6/nj790rtn62lfktb1sh__79hc0000gn/T/build-env-46q0ydet/lib/python3.11/site-packages/numpy/_core/include/numpy/ndarraytypes.h:368:24: note: expanded from macro 'PyArray_malloc'
#define PyArray_malloc PyMem_RawMalloc
                       ^
astropy/stats/src/fast_sigma_clip.c:109:19: warning: cast to 'double *' from smaller integer type 'int' [-Wint-to-pointer-cast]
    data_buffer = (double *)PyArray_malloc(n_i * sizeof(double));
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
astropy/stats/src/fast_sigma_clip.c:138:30: warning: cast to 'double *' from smaller integer type 'int' [-Wint-to-pointer-cast]
                mad_buffer = (double *)PyArray_malloc(n_i * sizeof(double));
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
astropy/stats/src/fast_sigma_clip.c:157:5: error: call to undeclared function 'PyMem_RawFree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    PyArray_free((void *)data_buffer);
    ^
/private/var/folders/y6/nj790rtn62lfktb1sh__79hc0000gn/T/build-env-46q0ydet/lib/python3.11/site-packages/numpy/_core/include/numpy/ndarraytypes.h:369:22: note: expanded from macro 'PyArray_free'
#define PyArray_free PyMem_RawFree
                     ^
astropy/stats/src/fast_sigma_clip.c:157:5: note: did you mean 'PyMem_Free'?
/private/var/folders/y6/nj790rtn62lfktb1sh__79hc0000gn/T/build-env-46q0ydet/lib/python3.11/site-packages/numpy/_core/include/numpy/ndarraytypes.h:369:22: note: expanded from macro 'PyArray_free'
#define PyArray_free PyMem_RawFree
                     ^
/Library/Frameworks/Python.framework/Versions/3.11/include/python3.11/pymem.h:55:18: note: 'PyMem_Free' declared here
PyAPI_FUNC(void) PyMem_Free(void *ptr);
                 ^
2 warnings and 2 errors generated.
error: command '/usr/bin/clang' failed with exit code 1

ERROR Backend subprocess exited when trying to invoke build_wheel

I can investigate this failure since this is in a module I wrote!

@astrofrog

Copy link
Copy Markdown
Member Author

Ok not quite right, investigating

@astrofrog
astrofrog force-pushed the limited-api-mac branch 2 times, most recently from 40c960a to d9e7d4b Compare June 10, 2025 10:06
@astrofrog
astrofrog requested a review from neutrinoceros June 10, 2025 10:08
@astrofrog
astrofrog marked this pull request as ready for review June 10, 2025 10:08
@astrofrog
astrofrog requested a review from larrybradley as a code owner June 10, 2025 10:08
@astrofrog

Copy link
Copy Markdown
Member Author

I think we can just use malloc/free here since these are just temporary arrays inside the C code and are never used by Python objects.

@astrofrog

Copy link
Copy Markdown
Member Author

@pllim - once this is merged we will need to update the branch protection rules since the CI name has changed for the limited API builds

@neutrinoceros

Copy link
Copy Markdown
Contributor

Hum, memory errors are showing up everywhere in CI now, so I wouldn't assume it's a simple drop-in replacement (at least not on every target platform ?) without studying more closely how the problematic PyArray APIs are defined.

@astrofrog

Copy link
Copy Markdown
Member Author

Ok curious, let me dig into this some more, I had it all passing locally but yes it must be sensitive to platform

astropy/table/setup_package.py \
astropy/utils/xml/setup_package.py \
astropy/wcs/setup_package.py
name: Remove incompatible extensions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this will create an unnecessary merge conflict with #18231

@astrofrog

Copy link
Copy Markdown
Member Author

Oh wait, commit issue..

@astrofrog
astrofrog force-pushed the limited-api-mac branch 3 times, most recently from 28015d0 to c66c932 Compare June 10, 2025 10:19
Comment thread astropy/stats/src/fast_sigma_clip.c Outdated
Comment thread astropy/stats/src/fast_sigma_clip.c Outdated
Comment thread astropy/stats/src/fast_sigma_clip.c Outdated
@astrofrog
astrofrog requested a review from neutrinoceros June 10, 2025 11:13
@astrofrog

Copy link
Copy Markdown
Member Author

@neutrinoceros - thanks for the review, I think this should be ready now

@neutrinoceros
neutrinoceros enabled auto-merge June 10, 2025 11:48
@neutrinoceros
neutrinoceros disabled auto-merge June 10, 2025 11:48
@neutrinoceros
neutrinoceros enabled auto-merge (squash) June 10, 2025 11:49
@neutrinoceros

Copy link
Copy Markdown
Contributor

Not clear from my perspective why auto-merge isn't going through. Is it related to branch protection rules ?

@astrofrog

Copy link
Copy Markdown
Member Author

@neutrinoceros - yes, the CI jobs have changed name here

@astrofrog
astrofrog disabled auto-merge June 10, 2025 12:47
@astrofrog
astrofrog merged commit 27c85f0 into astropy:main Jun 10, 2025
@astrofrog

Copy link
Copy Markdown
Member Author

I've updated the branch protection rules for main

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.

Platform-dependent issue with compiling against version 3.11 of Limited API

2 participants