Skip to content

BUG: Fixes #17797 f2py assumed shape and callbacks#17800

Closed
krystophny wants to merge 2 commits intonumpy:masterfrom
krystophny:master
Closed

BUG: Fixes #17797 f2py assumed shape and callbacks#17800
krystophny wants to merge 2 commits intonumpy:masterfrom
krystophny:master

Conversation

@krystophny
Copy link
Copy Markdown

This is a small fix for #17797 that checks if a custom __user__ use statement was inserted for callback functionality in f2py. I figured that func2subr.py was the best place to do these changes, as the generation happens there, and the code is already quite intertwined. Going directly into crackfortran.py instead would affect too many other things.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming line is a str and not a list: the second condition is a subset of the first one, and so is redundant - was that deliberate?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@eric-wieser thanks for the quick review! I have changed the condition to something more meaningful. Actually it should remove everything that looks like use something__user__stuff.

@krystophny
Copy link
Copy Markdown
Author

... just built scipy successfully with my modified numpy here, so seems not to break f2py also besides unit test here.

@krystophny
Copy link
Copy Markdown
Author

@eric-wieser any news on the topic? Is there anything else I should change? In addition I'm also planning to add a small patch on how to treat external syntax correctly, allowing statements real, external :: fcn in a single line according to the Fortran90 standard. I will open another bug on this and, if the PR is not approved until I fix it, add it here too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is __user__ an existing f2py thing, or something new you're adding here?

Copy link
Copy Markdown
Author

@krystophny krystophny Nov 27, 2020

Choose a reason for hiding this comment

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

This is an f2py thing and also checked in other places as an exceptional case. (see https://github.com/numpy/numpy/blob/master/doc/source/f2py/python-usage.rst )

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

... even more explicitly: This is the intended and documented behaviour of f2py that also occurs if you don't trigger the bug, i.e. don't define an assumed shape array in the same subroutine.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@eric-wieser what are the further steps now? As far as I can tell the PR fixes the bug and introduces only minimal changes to reproduce documented behaviour.

@pearu
Copy link
Copy Markdown
Contributor

pearu commented Jan 17, 2021

I'll review this PR. Please don't merge yet.

@pearu
Copy link
Copy Markdown
Contributor

pearu commented Jan 17, 2021

I confirm, this PR does not fix the underlying problem: when using an assumed shape array argument, the code must be f90, and wrapping the subroutine is required. So, f2py constructs a wrapper subroutine containing f90 code but the constructed code is compiled in f77 mode (notice the .f extension) which leads to the compilation failure.

This PR works around the problem by skipping __user__ module (which might be correct) but the underlying problem still persists. For instance, when the user subroutine would be using some other f90 module, the same compilation failure would be triggered because of the wrong compile mode.

@pearu pearu added 50 - Duplicate 57 - Close? Issues which may be closable unless discussion continued labels Jan 19, 2021
@krystophny
Copy link
Copy Markdown
Author

Superseeded by #18184 which includes the present fix regarding __user__.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug 50 - Duplicate 57 - Close? Issues which may be closable unless discussion continued

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants