Skip to content

BUG: fix f2py tests to work with v2 API #26935

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

Merged
merged 1 commit into from
Jul 17, 2024
Merged

BUG: fix f2py tests to work with v2 API #26935

merged 1 commit into from
Jul 17, 2024

Conversation

paddyroddy
Copy link
Contributor

@paddyroddy paddyroddy commented Jul 13, 2024

Fixes #26917. I've also modernised/improved the code in the relevant test files.

CI was failing prior to a4bd4a8 due to compiler errors in CI. Obviously not ideal reinstating this, but locally on my macOS 14.5 arm64 all tests pass with spin test -m full.

@paddyroddy paddyroddy marked this pull request as ready for review July 13, 2024 22:57
@paddyroddy paddyroddy marked this pull request as draft July 13, 2024 22:57
@paddyroddy paddyroddy marked this pull request as ready for review July 13, 2024 23:48
@seberg
Copy link
Member

seberg commented Jul 14, 2024

Yeah, what we would need is detecting whether compilation should work first, and then compile only (rather than just trying to compile the actual module).

@charris, I guess this is maybe the last thing for a 2.0.1?

@paddyroddy paddyroddy marked this pull request as draft July 14, 2024 16:33
@@ -115,7 +115,7 @@ static PyObject *f2py_rout_wrap_attrs(PyObject *capi_self,
PyArray_DESCR(arr)->type,
PyArray_TYPE(arr),
PyArray_ITEMSIZE(arr),
PyArray_DESCR(arr)->alignment,
PyDataType_ALIGNMENT(arr),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Core change

@@ -328,7 +342,7 @@ def build_meson(source_files, module_name=None, **kwargs):
# compiler stack is on the CI
try:
backend.compile()
except:
except subprocess.CalledProcessError:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better than a bare except

@paddyroddy paddyroddy marked this pull request as ready for review July 14, 2024 17:17
@seberg
Copy link
Member

seberg commented Jul 15, 2024

Thanks for fixing this, @paddyroddy seems I forgot to ask for that: But could you please undo the "non core" changes?
I wouldn't care about auto-fixing whitespaces on touched files, but sweeping auto fixing of things like list comprehensions should be a dedicated PR.

@seberg
Copy link
Member

seberg commented Jul 16, 2024

Seems f2py just isn't linted, see tools.linter.py. Dunno if that could be made more precise, I suspect there is a reason it isn't, but it could be that whitespace cleanups over time have made it more manageable.

EDIT: Sorry, wrong browser tab...

This is a clear bug which CI unfortunately doesn't see because it
just skips the f2py test if compilation fails (which it does here).

The error is slightly more precise now, but not precise enough to
avoid that issue.
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label Jul 17, 2024
@seberg
Copy link
Member

seberg commented Jul 17, 2024

Thanks, merging since we should backport this for 2.0.1. I did remove all the "non-core" changes from the PR, however.

@seberg seberg merged commit 776f881 into numpy:main Jul 17, 2024
67 of 68 checks passed
@charris charris changed the title BUG: fix f2py tests to work with v2 API BUG: fix f2py tests to work with v2 API Jul 17, 2024
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 19, 2024
@paddyroddy paddyroddy deleted the f2py-compile-test branch July 21, 2024 03:20
@paddyroddy
Copy link
Contributor Author

paddyroddy commented Jul 21, 2024

Thanks, merging since we should backport this for 2.0.1. I did remove all the "non-core" changes from the PR, however.

Cool, no problem. Sorry, I've been on holiday that past week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

numpy/f2py/tests/test_array_from_pyobj.py skips all tests due to use of legacy dtype API
3 participants