ENH: Support character and character string arrays #19388
Conversation
78ce316 to
0d27f21
Compare
|
Wow, that's an impressive list of things it fixes! Perhaps this PR is not reviewable in detail, since it's both large and complex code. Perhaps good to review the tests and main approach in detail, and then if it passes and SciPy build is happy as well, then just merge it? |
There was a problem hiding this comment.
I will not say I understand all the details :) I did take a look and one comment is that maybe we can make it clearer in the user guide when to use the new f2py_ macros (maybe just crosslinking the Advanced Usages and the Signature file guide.
I am also still investigating, but I think this breaks the SciPy tests. I'll check and see if I can pinpoint what is going on...
EDIT: Yes, this breaks the SciPy tests as character variables are not properly initialized. For example, running the SciPy tests, in subroutine _gesvx, in the flapack.pyf file, the variable fact should be initialized to "E", but it is instead being assigned the value E (note the lack of quotes!)
Here's a reproducer: saving the below as chartest.pyf
python module chartest
interface
subroutine chartest(fact, test)
character optional, intent(in) :: fact = 'E'
character, intent(out) :: test
end subroutine chartest
end interface
end python module chartest
generates chartestmodule.c with the following code:
/* Processing variable fact */
if (fact_capi == Py_None) fact = E; else
f2py_success = character_from_pyobj(&fact,fact_capi,"chartest.chartest() 1st keyword (fact) can't be converted to character");
if (f2py_success) {
/* Processing variable test */
It looks like character variables are being parsed as scalars and the initial value is going through _eval_scalar here
f42cc06 to
1601cce
Compare
|
There is another issue with scipy: consider https://github.com/scipy/scipy/blob/master/scipy/linalg/flapack_gen.pyf.src#L534 where the While this PR implements correct behavior for |
Is there a way to work around it, like special casing SciPy within character array handling code in f2py? |
|
The commit b176abe resolves the character BC issue by rewriting C-expressions after reading .pyf or Fortran files. |
|
Using this PR, the current scipy builds ok and all scipy tests pass. The PR fixes a number of long-lasting issues and blocks other f2py-related PRs. Unless there will be feedback that requires considerable effort, I'd like to land it as soon as possible, say, in 5 days. @melissawm @seberg @rgommers @eric-wieser @mattip @HaoZeke please review. |
TST: added test for issue numpy#18684 ENH: f2py opens files with correct encoding, fixes numpy#635 TST: added test for issue numpy#6308 TST: added test for issue numpy#4519 TST: added test for issue numpy#3425 ENH: Implement user-defined hooks support for post-processing f2py data structure. Implement character BC hook. ENH: Add support for detecting utf-16 and utf-32 encodings.
16406aa to
5bdeb35
Compare
0fc6adb to
bcbd02b
Compare
bcbd02b to
f20d90c
Compare
|
@melissawm @pearu I'm in favor of landing this ASAP, SciPy builds and all the tests pass. @charris, do you think the release notes are alright or should they be reworded? |
a3bd111 to
1d9952d
Compare
1d9952d to
5c0ecb5
Compare
This PR provides the following features:
support for wrapping Fortran functions with
character x)character, dimension(n) :: x)character(len=10) x)character(len=10), dimension(n, m) :: x)arguments, including passing Python unicode strings as Fortran character string arguments.
This support includes comprehensive tests and documentation.
the generated extension modules don't use the deprecated NumPy CAPI anymore
improved f2py generated exception messages
numerous bug and flake8 warning fixes
various CPP macros that one can use within C-expressions of signature files are prefixed with
f2py_. For example, one should usef2py_len(x)instead oflen(x)a new construct
character(f2py_len=...)is introduced to support returning assumed length character strings (e.g.character(len=*)) from wrapper functionsintroduce a hook to support rewriting f2py internal data structures after reading all its input files. This is required, for instance, for BC of scipy support where character arguments are treated as character strings arguments in C expressions.
Fixes #18684, #635, #6308, #4519, #3425, #10027