DOC: Updating f2py docs to python 3 and fixing some typos #15303

melissawm · 2020-01-09T21:36:26Z

Hi all,

This PR fixes problems with the f2py documentation. I added some comments from the discussion here to the docs to make it clearer - feel free to remove if you think it's not necessary.

Closes #14865
Closes #14862
Closes #14812
Closes #14960

…ck for string bug here.

…on-usage.rst

…2py-docs-python3 * 'f2py-docs-python3' of github.com:melissawm/numpy: Fixed typo. Fixed typos. Remove future imports from f2py example. Finished update of usage.rst PEP8 fix for equal sign and added note about building extension modules. WIP: updating usage.rst Finished update of python-usage.rst WIP: updated sections "Common blocks" and "F90 module data" from python-usage.rst WIP: updated "Callback arguments" section in python-usage.rst WIP: updated "Array arguments" section of python-usage.rst WIP: updated "String arguments" section of python-usage.rst. TODO check for string bug here. WIP: updated "Scalar Arguments" session of python-usage.rst WIP: updating f2py docs to python3: intro to python-usage done. Updated f2py "Getting Started" doc to python3.

mattip

Looks much better now. I made a few small comments

mattip · 2020-01-09T23:41:22Z

doc/source/f2py/allocarr_session.dat

@@ -14,7 +15,9 @@ foo - Function signature:
 ]
 >>> allocarr.mod.b                             # b is Fortran-contiguous
 array([[ 1.,  2.,  3.],
-       [ 4.,  5.,  6.]],'f')
+       [ 4.,  5.,  6.]], dtype=float32)


float32 -> np.float32 and add import numpy as np to the top of the file

Thanks for all the comments! However this is the actual output from the allocarr module, so I can't change that. Maybe fixing this should be an issue for f2py?

The output of print(allocarr.mod.__doc__) is wrong. I haven't look into the rest of the changes but this indicates some bug.

It is more of a numpy issue how the array is displayed in repr mode.

doc/source/f2py/allocarr_session.dat

doc/source/f2py/array_session.dat

mattip · 2020-01-09T23:45:09Z

doc/source/f2py/calculate_session.dat

@@ -3,4 +3,4 @@
 array([  0.,   1.,   4.,   9.,  16.])
 >>> import math
 >>> foo.calculate(range(5), math.exp)
-array([  1.        ,   2.71828175,   7.38905621,  20.08553696,  54.59814835])
+array([  1.        ,   2.71828183,   7.3890561,  20.08553692,  54.59815003])


any idea why the results changed?

No idea, it surprised me too!

It looks like using float32 changed to float64 or vice-versa in the underlying code.

It could be due to defining real*8 func. Before the type of func was real.

Actually, func was not defined in the .f file so this example was not working. See #14919 . In fact, declaring func as real gives this output:

>>> import foo >>> foo.calculate(range(5), lambda x: x*x) array([0., 0., 0., 0., 0.]) >>> import math >>> foo.calculate(range(5), math.exp) array([ 0.00000000e+00, -2.85695233e-32, -1.01502388e-04, -9.36294913e-01, 3.80232235e-11]) >>>

This example only works if we declare func as real*8

mattip · 2020-01-09T23:47:16Z

doc/source/f2py/ftype_session.dat

@@ -18,4 +18,4 @@ COMMON blocks:
 >>> ftype.foo(24)
 IN FOO: N= 24 A=  3. X=[  1.  45.  3.]
 >>> ftype.data.x
-array([  1.,  45.,   3.],'f')
+array([  1.,  45.,   3.], dtype=float32)


float32 -> np.float32, and import numpy as np` at the top of the file

This is the same case as above, it's the output from the module generated by f2py, so I can't change that.

I think @mattip is wrong here, and misread what this code was.

doc/source/f2py/getting-started.rst

mattip · 2020-01-09T23:50:09Z

doc/source/f2py/moddata_session.dat

@@ -20,4 +21,6 @@ foo - Function signature:
 Setting a(1,2)=a(1,2)+3
 >>> moddata.mod.a               # a is Fortran-contiguous
 array([[ 1.,  5.,  3.],
-       [ 4.,  5.,  6.]],'f')
+       [ 4.,  5.,  6.]], dtype=float32)


Again, this is output from the f2py generated module.

Or more generally, this is just now arrayprint.array_repr works.

mattip · 2020-01-10T00:11:37Z

doc/source/f2py/string_session.dat

 A=123
- B=123
+ B=123��


I think this now need a PEP-263 line:

# -*- coding: utf-8 -*-

at the top of the file

I think this is an open issue with f2py, because this is the output that we get from the generated module. See #4519 . I'm not sure if I should leave the docs like and include maybe a note about this being open, or write the expected output (but then the docs would be misleading). Any ideas?

@mattip, this isn't a python file, so that won't help.

It's possible that github is making assumption based on the .dat file extension - renaming to txt might help, as might adding a .gitattributes entry.

The result for B is actually expected: foo's second argument is expected to have 5 character input but the user gives less. As a quick fix, try b = array('123\0')

but the user gives less

Shouldn't this be an error? Or if not, should python pad with nulls?

Yes, f2py should trigger a ValueError when the input has a smaller byte-size than expected by array argument.
@melissawm , can you create another issue on this? To resolve the given example issue, use b = array('123\0\0') so that it will not break when the bug will be fixed.

@pearu using b = array('123\0\0') gives me the following:

>>> from numpy import array >>> a = array('123') >>> b = array('123\0\0') >>> c = array('123') >>> d = array('123') >>> mystring.foo(a, b, c, d) A=1 B=1 C=1 D=1 CHANGE A,B,C,D A=A B=B C=C D=D >>> a.tostring(), b.tostring(), c.tostring(), d.tostring() (b'1\x00\x00\x002\x00\x00\x003\x00\x00\x00', b'B \x00', b'1\x00\x00\x002\x00\x00\x003\x00\x00\x00', b'D \x00')

I can open another issue for the lack of ValueError when the array is too small, but I think this is related to #4519

Use

a = array(b'123\0\0') b = array(b'123\0\0') c = array(b'123') d = array(b'123')

f2py might be unicode ignorant..

That solves it. The issue about not raising the ValueError is here #15311

You may as well remove .tostring() (which is really ndarray.tobytes()) - if you want just the item in the array, you can use a[()], b[()], c[()], d[()], which will also hide the trailing nulls for you

doc/source/f2py/usage.rst

doc/source/f2py/common_session.dat

doc/source/f2py/moddata_session.dat

doc/source/f2py/usage.rst

pearu

I left some comments. Overall looks good.

I am worried about the fact that the generated documentation strings of common blocks and module data do not contain the names of the items. These are needed! This indicates a bug that has been introduced to f2py in the past.

pearu · 2020-01-10T14:23:40Z

doc/source/f2py/moddata_session.dat

-foo - Function signature:
-  foo()
+>>> print(moddata.mod.__doc__)
+'i'-scalar


The names of module data should be restored.

Same as above, depends on #15325

pearu · 2020-01-10T14:31:38Z

doc/source/f2py/string_session.dat

 A=123
- B=123
+ B=123��


The result for B is actually expected: foo's second argument is expected to have 5 character input but the user gives less. As a quick fix, try b = array('123\0')

pearu · 2020-01-10T14:33:05Z

doc/source/f2py/usage.rst

@@ -21,7 +22,7 @@ Command ``f2py``
 =================

 When used as a command line tool, ``f2py`` has three major modes,
-distinguished by the usage of ``-c`` and ``-h`` switches:
+distinguished by the usage of ``-c``, ``-m`` and ``-h`` switches:


-m does not define a mode. It just specifies the name of the generated module. Suggest undo.

Btw, when -m is not used, then it is equivalent to usage -m untitled

doc/source/f2py/allocarr_session.dat

pearu · 2020-01-10T14:35:53Z

doc/source/f2py/allocarr_session.dat

@@ -14,7 +15,9 @@ foo - Function signature:
 ]
 >>> allocarr.mod.b                             # b is Fortran-contiguous
 array([[ 1.,  2.,  3.],
-       [ 4.,  5.,  6.]],'f')
+       [ 4.,  5.,  6.]], dtype=float32)


It is more of a numpy issue how the array is displayed in repr mode.

doc/source/f2py/allocarr_session.dat

pearu · 2020-01-10T14:39:24Z

doc/source/f2py/calculate_session.dat

@@ -3,4 +3,4 @@
 array([  0.,   1.,   4.,   9.,  16.])
 >>> import math
 >>> foo.calculate(range(5), math.exp)
-array([  1.        ,   2.71828175,   7.38905621,  20.08553696,  54.59814835])
+array([  1.        ,   2.71828183,   7.3890561,  20.08553692,  54.59815003])


It could be due to defining real*8 func. Before the type of func was real.

doc/source/f2py/usage.rst

numpygh-14960; fixes numpygh-14865; fixes numpygh-14862

melissawm

I think I addressed all comments, let me know if any other changes are necessary.

pearu

Added just one minor request, otherwise LGTM.

doc/source/f2py/common_session.dat

eric-wieser · 2020-01-14T14:40:22Z

I'm not sure I understand why we're changing the terminal sessions to indicate what we want to happen - shouldn't they indicate what actually happens, even if it turns out to be a bug?

rgommers · 2020-01-14T17:43:17Z

I'm not sure I understand why we're changing the terminal sessions to indicate what we want to happen - shouldn't they indicate what actually happens, even if it turns out to be a bug?

documenting buggy behavior sounds odd and is not what we normally do. CI is green, so there's no reason not to document the correct behavior, which will match actual behavior again once the now identified regression is fixed. right?

charris · 2020-01-19T18:27:54Z

once the now identified regression is fixed. right?

Has it been fixed?

eric-wieser · 2020-01-19T20:35:03Z

No, it has not been fixed, and that is what makes me uneasy about this patch.

CI is green, so there's no reason not to document the correct behavior,

Well, CI used to be green for all documentation until @mattip turned on refguide. Just because CI isn't currently forcing us to make our docs accurate doesn't mean we shouldn't.

pearu · 2020-01-19T21:10:42Z

Just because CI isn't currently forcing us to make our docs accurate doesn't mean we shouldn't.

In this case, the code is faulty, not the docs. Already this fact should force us to fix the code instead of making the docs faulty as well. (This is a good example where accurate behavior can be a wrong behavior).

charris · 2020-01-19T21:24:15Z

OK, let's put this in and make sure there is an issue for the fix. There are a ton of open issues for f2py that we should triage.

charris · 2020-01-19T21:25:51Z

Thanks Melissa.

melissawm added 19 commits November 19, 2019 12:54

Updated f2py "Getting Started" doc to python3.

0e8053c

WIP: updating f2py docs to python3: intro to python-usage done.

a39782e

WIP: updated "Scalar Arguments" session of python-usage.rst

7f17a7e

WIP: updated "String arguments" section of python-usage.rst. TODO che…

b76fa2a

…ck for string bug here.

WIP: updated "Array arguments" section of python-usage.rst

bc4e2ce

WIP: updated "Callback arguments" section in python-usage.rst

f3804b2

WIP: updated sections "Common blocks" and "F90 module data" from pyth…

3afb671

…on-usage.rst

Finished update of python-usage.rst

8d217b0

WIP: updating usage.rst

7edc44a

PEP8 fix for equal sign and added note about building extension modules.

41bd076

Finished update of usage.rst

3ca300e

Remove future imports from f2py example.

d4ea6bc

Fixed typos.

0cd0568

Fixed typo.

377bcf7

Updated f2py "Getting Started" doc to python3.

55264af

Finished update of python-usage.rst

a6159f6

Finished update of usage.rst

6ea8f3b

Fixed typos.

9a9aa97

mattip reviewed Jan 10, 2020

View reviewed changes

Addressing comments on PR.

edb5f56

pearu reviewed Jan 10, 2020

View reviewed changes

doc/source/f2py/common_session.dat Outdated Show resolved Hide resolved

eric-wieser reviewed Jan 10, 2020

View reviewed changes

doc/source/f2py/moddata_session.dat Outdated Show resolved Hide resolved

eric-wieser reviewed Jan 10, 2020

View reviewed changes

doc/source/f2py/usage.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Jan 10, 2020

View reviewed changes

doc/source/f2py/usage.rst Outdated Show resolved Hide resolved

pearu requested changes Jan 10, 2020

View reviewed changes

Addressing PR review; closes numpygh-14812; fixes numpygh-14919; closes

09a8f2b

numpygh-14960; fixes numpygh-14865; fixes numpygh-14862

melissawm commented Jan 14, 2020

View reviewed changes

pearu approved these changes Jan 14, 2020

View reviewed changes

doc/source/f2py/common_session.dat Outdated Show resolved Hide resolved

Restore names of common block items.

eeb56d6

rgommers added 04 - Documentation component: numpy.f2py labels Jan 14, 2020

charris merged commit 73d8ec8 into numpy:master Jan 19, 2020

melissawm deleted the f2py-docs-python3 branch January 20, 2020 11:50

DOC: Updating f2py docs to python 3 and fixing some typos #15303

DOC: Updating f2py docs to python 3 and fixing some typos #15303

Conversation

melissawm commented Jan 9, 2020 • edited by charris

mattip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Jan 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-wieser Jan 10, 2020 • edited

Choose a reason for hiding this comment

pearu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melissawm left a comment

Choose a reason for hiding this comment

pearu left a comment

Choose a reason for hiding this comment

eric-wieser commented Jan 14, 2020

rgommers commented Jan 14, 2020

charris commented Jan 19, 2020

eric-wieser commented Jan 19, 2020

pearu commented Jan 19, 2020 via email • edited

charris commented Jan 19, 2020

charris commented Jan 19, 2020

melissawm commented Jan 9, 2020 •

edited by charris

eric-wieser Jan 10, 2020 •

edited

eric-wieser Jan 10, 2020 •

edited

pearu commented Jan 19, 2020 via email •

edited