Skip to content

MAINT: f2py cleanup#22885

Merged
seberg merged 3 commits intonumpy:mainfrom
HaoZeke:cleanCrackfortran
Jan 16, 2023
Merged

MAINT: f2py cleanup#22885
seberg merged 3 commits intonumpy:mainfrom
HaoZeke:cleanCrackfortran

Conversation

@HaoZeke
Copy link
Copy Markdown
Member

@HaoZeke HaoZeke commented Dec 25, 2022

Updates the free format handling of .f90 and other common extensions (through a minor re-write). Also removes an unused function.

This disallows previously allowed (but highly unlikely to be present) code-paths, namely having fixed form F77 code in a fortran 90 file (with .f90).

@HaoZeke HaoZeke requested a review from melissawm December 25, 2022 20:13
@HaoZeke HaoZeke mentioned this pull request Dec 25, 2022
Merged
2 tasks
@HaoZeke HaoZeke requested a review from pearu December 25, 2022 20:22
seberg
seberg reviewed Jan 4, 2023
View reviewed changes
@HaoZeke @seberg
MAINT: Remove is_f_file
b68d802 
Co-authored-by: Sebastian Berg <sebastianb@nvidia.com>
@seberg
Copy link
Copy Markdown
Member

seberg commented Jan 16, 2023 •
edited
Loading

Well, the days of this code are counted, it looks fine, and I will trust you follow up if something breaks. So lets put this in, thanks @HaoZeke. (the distutils mainly is ugly with the \, but that code is de-facto end-of-life anyway, so I will not bother today)

@seberg seberg merged commit 6b5cd92 into numpy:main Jan 16, 2023
@seberg
Copy link
Copy Markdown
Member

seberg commented Jan 16, 2023

To be clear, I am assuming that the change:

This disallows previously allowed (but highly unlikely to be present) code-paths, namely having fixed form F77 code in a fortran 90 file (with .f90).

Does seemingly not worry the f2py specialists here.

rgommers added a commit to rgommers/numpy that referenced this pull request Jan 22, 2023
@rgommers
BUG: fix broken numpy.distutils Fortran handling
5437558 
The `Path` and `COMMON_FIXED_EXTENSIONS` variables were not
defined at all. This code is untested, and SciPy doesn't build
with NumPy `main` before this fix. The issue was introduced
a few days ago in numpygh-22885.
@rgommers rgommers mentioned this pull request Jan 22, 2023
Merged
@rgommers
Copy link
Copy Markdown
Member

rgommers commented Jan 22, 2023

The numpy.distutils changes in this PR are completely broken, Path and COMMON_FIXED_EXTENSIONS are not defined. This code is both deprecated and untested, so please avoid touching it. And if you really have to, please test that you do not break SciPy.

@rgommers
Copy link
Copy Markdown
Member

rgommers commented Jan 22, 2023

I'll merge gh-23066 once CI passes and will trigger new nightly builds, to unbreak SciPy CI.

@mattip
Copy link
Copy Markdown
Member

mattip commented Jan 22, 2023

I wonder why coverage did not mark the lack of tests here.

@rgommers
Copy link
Copy Markdown
Member

rgommers commented Jan 22, 2023

It probably does show that. There are very few tests for numpy.distutils, and branch coverage is even worse.

rgommers added a commit that referenced this pull request Jan 22, 2023
@rgommers
Revert "MAINT: f2py cleanup (#22885)"
a68f24b 
This reverts commit 6b5cd92.
@rgommers rgommers mentioned this pull request Jan 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seberg seberg seberg left review comments

@melissawm melissawm Awaiting requested review from melissawm

@pearu pearu Awaiting requested review from pearu

Assignees

No one assigned

Labels

03 - Maintenance component: numpy.f2py

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HaoZeke @seberg @rgommers @mattip