Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BUG: Fix F77 ! comment handling #26537

Merged
merged 6 commits into from
Jun 2, 2024
Merged

BUG: Fix F77 ! comment handling #26537

merged 6 commits into from
Jun 2, 2024

Conversation

HaoZeke
Copy link
Member

@HaoZeke HaoZeke commented May 26, 2024 •
edited
Loading

Closes #26466. Closes #26148.

They're essentially rooted in the same problem, that is, incorrect handling in strictf77 mode for ! comments.

This PR:

@HaoZeke HaoZeke requested review from charris and mattip May 26, 2024 17:38
@HaoZeke
Copy link
Member Author

HaoZeke commented May 26, 2024 •
edited
Loading

To be fair here, though, ! in F77 is an extension. However, it seems rather likely to be in use in these legacy code-bases so we might as well support it.
image

Note that G77 has some additional ideas about what ! can mean which are likely to be missed here:
image

In particular, it seems that F77 codes relying on using ! in the sixth column will now have trouble (which is a regression), so I'll make this a draft until that's fixed.

@HaoZeke HaoZeke marked this pull request as draft May 26, 2024 18:02
@HaoZeke HaoZeke marked this pull request as ready for review May 26, 2024 18:34
@HaoZeke
Copy link
Member Author

HaoZeke commented May 26, 2024 •
edited
Loading

After some playing around, it seems like ! in the character columns is typically interpreted as a comment anyway by gfortran by default, and even before this PR having ! in the continuation column would not do what is expected (i.e. it seems to be treated as a comment), so on whole this should probably go in as is, and can be iterated on if other issues come up.

HaoZeke and others added 6 commits May 26, 2024 20:49
@HaoZeke @xiangyi-wang
@RibomBalt @gmloose
TST: Add minimal reproducer for numpygh-26148
c7d3acd 
Co-authored-by: xiangyi-wang <xiangyi-wang@users.noreply.github.com>
Co-authored-by: RibomBalt <RibomBalt@users.noreply.github.com>
Co-authored-by: gmloose <gmloose@users.noreply.github.com>
@HaoZeke
MAINT: Fix spelling and formatting issue [f2py]
59d133f
@HaoZeke
BUG: Fix comment handling for ! in F77
3e1b67a 
Closes numpygh-26148
@HaoZeke
MAINT: Cleanup now unnecessary strictf77 path
0a3d0b1
@HaoZeke
TST: Ensure simplified strictf77 path works
53e64c3
@HaoZeke
TST: Add one for numpygh-26466
0888577 
Closes numpygh-26466

Since that is also related to comment parsing in F77
@HaoZeke HaoZeke requested a review from melissawm May 28, 2024 19:41
@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Jun 2, 2024
@charris charris merged commit acefc26 into numpy:main Jun 2, 2024
66 checks passed
@charris
Copy link
Member

charris commented Jun 2, 2024

Thanks Rohit. I guess we will find out if this causes any problems.

@HaoZeke HaoZeke deleted the fixF77comments_26148 branch June 2, 2024 21:44
@charris charris mentioned this pull request Jun 9, 2024
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jun 9, 2024
@HaoZeke HaoZeke restored the fixF77comments_26148 branch June 15, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charris charris Awaiting requested review from charris

@mattip mattip Awaiting requested review from mattip

@melissawm melissawm Awaiting requested review from melissawm

Assignees
No one assigned
Labels
00 - Bug component: numpy.f2py
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: AssertError from f2py in crackfortran.py BUG: f2py Fortran77 comments "!" parsing error for subroutine
2 participants
@HaoZeke @charris