This looks like it really is all there is to it? Can you just copy the failing example from the issue as a test?

This looks like it really is all there is to it? Can you just copy the failing example from the issue as a test?

It did fix the issue, but then I noticed some of the other parts of the code depend on hard-coded values of this counter (e.g. generating wrapper files), so incrementing the counter probably introduces additional bugs. Will take a closer look soon.

A better fix would be to change the variable names so the block Fortran 2008 construct can be recognized within crackfortran, however since there's no support for it anyway, and it would be a rather large change, this seems like the best approach for now.

@charris this bug is present from 1.21.0 onwards, including the release candidates, and the fix might need a backport?

ping @pearu or anyone else from the f2py team for a review

Thank you for this PR!

I did a crude check of this PR on my setup. Crude means: I did not install this version of numpy, but I used my existing numpy installation 1.23.5 and replaced the folder f2py with the one from this PR. I used this workaround successfully before, using the f2py folder from numpy 1.19.5, which allows me to compile our project.

Unfortunately, this patch here does not seem fix everything. It definitely fails less, but I still get an error on one of our fortran codes:

Line #61 in CMakeFiles/_phojet191.dir/PHO_PRIMKT.f:"      PARAMETER (IRMAX=200)"
	get_parameters: got "eval() arg 1 must be a string, bytes or code object" on 4
	Reading file 'CMakeFiles/_phojet191.dir/PHO_PRSTRG.f' (format:fix,strict)
	Reading file '/Users/hdembinski/Extern/chromo/src/fortran/dpmjetIII-19.1/include/phojet/inc/poinou' (format:fix)
	Reading file '/Users/hdembinski/Extern/chromo/src/fortran/dpmjetIII-19.1/include/phojet/inc/podebg' (format:fix)
	Reading file '/Users/hdembinski/Extern/chromo/src/fortran/dpmjetIII-19.1/include/phojet/inc/poevt1' (format:fix)
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.10/3.10.9/Frameworks/Python.framework/Versions/3.10/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.10/3.10.9/Frameworks/Python.framework/Versions/3.10/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/__main__.py", line 5, in <module>
    main()
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/f2py2e.py", line 704, in main
    run_main(sys.argv[1:])
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/f2py2e.py", line 441, in run_main
    postlist = callcrackfortran(files, options)
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/f2py2e.py", line 342, in callcrackfortran
    postlist = crackfortran.crackfortran(files)
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/crackfortran.py", line 3300, in crackfortran
    readfortrancode(files, crackline)
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/crackfortran.py", line 552, in readfortrancode
    dowithline(finalline)
  File "/Users/hdembinski/Extern/chromo/venv/lib/python3.10/site-packages/numpy/f2py/crackfortran.py", line 819, in crackline
    raise Exception('crackline: groupcounter(=%s) is nonpositive. '
Exception: crackline: groupcounter(=0) is nonpositive. Check the blocks.

Sorry, I cannot give a minimal reproducer, the fortran codes which we wrap are very complex and I am not an expert. If you want to try it out yourself, you can get the code from git@github.com:impy-project/chromo.git. Compile with python setup.py develop. You need cmake, setuptools, wheel, numpy and gcc/gfortran. Before compiling you will need to delete all files in the folder src/f2py, so that they are re-generated. We started to cache these files to work around the numpy.f2py bug, but that workaround has its own set of issues, so I really want to move away from that again once this is fixed.

close/reopen

I would like to test this PR locally on chromo, but when I try to check this branch out locally, I get this error

From https://github.com/numpy/numpy
 ! [rejected]            refs/pull/22657/head -> fix22648  (non-fast-forward)
failed to run git: exit status 1

I am not sure how this fix.

Edit: Nevermind, I got it to work. I cloned numpy again from scratch.

Firstly, I am extremely grateful to @HaoZeke for trying to figure this out.

We are testing this PR over here in our project impy-project/chromo#127

Unfortunately the latest version still does not work on the module _phojet191 that we build among other things. I provided instructions over at impy-project/chromo#127 on how to build just this offending module (we normally build many more), to simplify testing/debugging this issue.

Needs a rebase. @HaoZeke @pearu Thoughts on getting the problem fixed?

@HDembinski @afedynitch could you test this?

@HaoZeke I think a rebase will fix the test failure on azure.

We are currently testing it, much thanks to @HaoZeke

Side-comment: considering how complex the crackfortran code is, it may be a good target for a rewrite using a formal parser. I personally had only good experiences with the lark library in Python, which allows you to specify the rules in EBNF notation. Lark can generate a standalone parser in Python from this spec, so client code does not even need the lark library as a dependency. I just leave this here as a note.

@HDembinski Any success? I'd like to get this in before the 1.25 release if it works.

@charris In most cases, this seems to work. We identified a problem that happens only on Windows, whereas the same code parses well on mac and linux, and was parsing fine on pre-1.20 on Windows. Also, we have seem some changes to the interface that were easy to fix but we were wondering if they are intentional, such as a difference the interpretation of character* arrays.

Except that Windows problem, which I believe is a bug, the other issues are not a problem for us but may break some backward compatibility.

@HaoZeke Ping.

@HDembinski Would it be helpful to put this in as is?

@afedynitch is currently handling this so I don't have the full overview. As far as I understand, we may be able to deal with it on our end by making code changes, but it is still a regression: the code we had is parsed in a different way and that requires adaptations in both the legacy Fortran and in the Python code.

@HDembinski Would it be helpful to put this in as is?

Yes, the present version solves most of our problems. But the windows issue might be a bug that you wouldn't want to put in, I guess. Maybe this is some sort of encoding issue, but it wasn't there with previous versions. Better if @HaoZeke can look at it shortly once more.

UPDATE: @charris It doesn't work on Windows. Most codes are missing the common block wrappers in -f2pywrappers.f

Sounds like there are still some problems. @HaoZeke Could you revisit this?

@pearu Thoughts?

@HaoZeke @charris @pearu Any new thoughts or progress on this?

@HaoZeke @charris @pearu Any new thoughts or progress on this?

I think the windows issue should be reported separately, and we should go ahead with this. There are some other open PRs with fixes for f2py which might also help windows, but none specifically (no reproducible bug reports yet).

@HaoZeke Could you relate to the comment that

[W]e have seem some changes to the interface that were easy to fix but we were were wondering if they are intentional, such as a difference the interpretation of [char* arrays](impy-project/chromo#127 (comment).

@HaoZeke Could you relate to the comment that

[W]e have seem some changes to the interface that were easy to fix but we were were wondering if they are intentional, such as a difference the interpretation of [char* arrays](impy-project/chromo#127 (comment).

The change in interpretation was part of #19388 and is also related to #24008 which should be fixed by #24299. These are however separate from the common block handling and shouldn't be covered here.

Thanks @HaoZeke

I think the windows issue should be reported separately, and we should go ahead with this. There are some other open PRs with fixes for f2py which might also help windows, but none specifically (no reproducible bug reports yet).

Thanks @HaoZeke for fixing the issue!

Indeed, you were right about the Windows bug being unrelated. It is cause by f2py --include-paths not picking up the colon separators for the multiple include paths required by our code. I filed it separately #24533 .

On a different note... is this fix going to be backported?

On a different note... is this fix going to be backported?

I think that would be a good idea, @charris ?

It was backported yesterday.