BLD: require extension-helpers 1.2.0 and simplify path type casting (⏰ wait for #16936)#17193
BLD: require extension-helpers 1.2.0 and simplify path type casting (⏰ wait for #16936)#17193neutrinoceros wants to merge 2 commits intoastropy:mainfrom
Conversation
Co-authored-by: Mridul Seth <git@mriduls.com>
|
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.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
mhvk
left a comment
There was a problem hiding this comment.
This looks good to me. Why does this need the extension helpers update, though?
| get_wcslib_cfg(cfg, wcslib_files, wcslib_config_paths) | ||
|
|
||
| cfg["include_dirs"].append(join(WCSROOT, "include")) | ||
| cfg["include_dirs"].append(WCSROOT / "include") |
There was a problem hiding this comment.
Should this have a str(...) too, like the others?
There was a problem hiding this comment.
ah, yes. I got things mixed up here, but passing Paths directly to setuptools requires another upgrade that we cannot afford yet because it's not available on exotic archs.
Anyway, thanks for catching this !
There was a problem hiding this comment.
Hmm, that leaves me still confused what part actually is solved by upgrading extension helpers.
There was a problem hiding this comment.
it should be clearer if you look just at the second commit
8b4fc9f (#17193)
| if isinstance(header, (str, bytes, os.PathLike)): | ||
| try: | ||
| is_path = os.path.exists(header) | ||
| is_path = os.path.exists(header) # noqa: PTH110 |
There was a problem hiding this comment.
Can one not replace with Path(header).exists()?
There was a problem hiding this comment.
that would be a comment for #16936, but yes, we could avoid a noqa here
See #16936 (comment) : extension-helpers 1.2.0 improves native support for |
|
OK, so then all the |
|
No. The remaining ones are needed for compatibility with setuptools<72, which we still need to support building on "exotic archs". |
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary. If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
Description
Extracted from and a continuation of #16936, cleaning up path type casting that becomes redundant with extension-helpers 1.2.0
This will require extension-helpers 1.2.0 or newer to become available on exotic archs.
update: Here's a dynamic view of
extension-helperspackaging status