Skip to content

BLD: fix an implicit conversion error (char[2] -> int)#155

Merged
mhvk merged 1 commit intoliberfa:mainfrom
neutrinoceros:bld/fixup_build_conversion_error
Aug 13, 2024
Merged

BLD: fix an implicit conversion error (char[2] -> int)#155
mhvk merged 1 commit intoliberfa:mainfrom
neutrinoceros:bld/fixup_build_conversion_error

Conversation

@neutrinoceros
Copy link
Contributor

Tentatively fix a build error seen in CI
example logs

I couldn't reproduce this locally yet so fingers crossed.

@neutrinoceros neutrinoceros force-pushed the bld/fixup_build_conversion_error branch from 6b62c6e to 4d8830c Compare August 13, 2024 08:02
@neutrinoceros neutrinoceros changed the title BLD: fix a conversion error (char[2] -> int) BLD: fix a conversion error (char[2] -> int) Aug 13, 2024
@neutrinoceros neutrinoceros force-pushed the bld/fixup_build_conversion_error branch 2 times, most recently from 4534bcf to 6f93a91 Compare August 13, 2024 08:07
@neutrinoceros neutrinoceros changed the title BLD: fix a conversion error (char[2] -> int) BLD: fix an implicit conversion error (char[2] -> int) Aug 13, 2024
@neutrinoceros neutrinoceros force-pushed the bld/fixup_build_conversion_error branch from 6f93a91 to e2a2d78 Compare August 13, 2024 08:16
@neutrinoceros
Copy link
Contributor Author

Ok I think I got it this time: wheel builds is green on macOS arm64.
Meanwhile, we're seeing another, unrelated error on windows: following upstream changes in cibuildwheel, our wheels are being tested on CPython 3.13 too, but it doesn't quite work since numpy doesn't have stable wheels for it yet. I'll fix this in another PR.

@neutrinoceros
Copy link
Contributor Author

ping @mhvk for review

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.

Thanks for finding that! Since get_liberfa_versions() actually returns the right types, it may make sense to just use that information, which would be a smaller change. Suggestions inline...

print('Configure liberfa ("configure.ac" scan)')
lines = []
for name, value in get_liberfa_versions():
lines.append(f'#define {name} "{value}"')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be done more simply as,

if isinstance(value, str):
    # strings should be escaped
     value = f'"{value}"'
lines.append(f'#define {name} {value}')

Or, just accounting for the fact that repr gives the wrong type of quote,

lines.append(f'#define {name} {value!r}'.replace("'", '"'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

neat. I just pushed your one-liner. Thanks !

@neutrinoceros neutrinoceros force-pushed the bld/fixup_build_conversion_error branch from e2a2d78 to 8d06124 Compare August 13, 2024 15:14
@mhvk
Copy link
Contributor

mhvk commented Aug 13, 2024

Great, let's get this in!

@mhvk mhvk merged commit 7e6397a into liberfa:main Aug 13, 2024
@neutrinoceros neutrinoceros deleted the bld/fixup_build_conversion_error branch August 13, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants