Skip to content

Conversation

@HaoZeke
Copy link
Member

@HaoZeke HaoZeke commented Aug 28, 2021

OK. This is ready now. It is essentially a very minor PR to make the code seem less out of date in production by:

  • Updating dead URLs to point to valid Archive links
  • Cleaning up some docstrings
  • Removing tests which don't run
  • Removing other parts of the code which are not relevant to numpy
    Let me know if this is too aggressive.

Closes #19910.

@HaoZeke HaoZeke force-pushed the minor_typos_dated branch 2 times, most recently from 3022b0f to deb2493 Compare August 30, 2021 21:43
@HaoZeke
Copy link
Member Author

HaoZeke commented Aug 30, 2021

RE: Removing tests

^ Another seemingly potentially aggressive cleaning step, this is a good thing to have before we move forward with the new test suite.

@HaoZeke HaoZeke force-pushed the minor_typos_dated branch 4 times, most recently from fe71379 to f9bdf76 Compare September 15, 2021 23:56
@HaoZeke HaoZeke changed the title WIP, MAINT: Readability improvements and cleanup [f2py] MAINT: Readability improvements and cleanup [f2py] Sep 15, 2021
@HaoZeke HaoZeke marked this pull request as ready for review September 15, 2021 23:57
@HaoZeke HaoZeke requested review from melissawm and pearu September 15, 2021 23:58
@HaoZeke HaoZeke removed the 25 - WIP label Sep 16, 2021
@HaoZeke HaoZeke changed the title MAINT: Readability improvements and cleanup [f2py] MAINT: Readability improvements and cleanup for f2py Sep 20, 2021
@HaoZeke HaoZeke changed the title MAINT: Readability improvements and cleanup for f2py MAINT,DOC: Readability improvements and cleanup for f2py Sep 20, 2021
@charris
Copy link
Member

charris commented Sep 21, 2021

RE: Removing tests

Are they currently unused? Are you putting together another test set?

@HaoZeke
Copy link
Member Author

HaoZeke commented Sep 21, 2021

RE: Removing tests

Are they currently unused? Are you putting together another test set?

This set is unused. There are tests under numpy/f2py/tests which are run with runtests.py. There are plans to update these for consistency; basically to extract the fortran snippets into their own file instead of keeping them in the python code.

Here is an extract of the logic behind this:

At the moment it seems like the newer tests use code and suffix to put the fortran source inside the test_*.py (8336e76#diff-beaa766b50d9c2a8df83b293fa468e786d9ffb8b10d3f2a44d46d81084abd022)

However, the older tests had (IMO) a nicer structure with the source code in a separate f/90 file (even though the older test style can be cleaned up now).

As an example of the older test approach it is the one in test_mixed.py or test_common.py (8042526).
For new tests, I'm thinking the split .f90 and test_*.py style should be used (with pathlib instead of the _path helper)

@HaoZeke HaoZeke requested a review from charris September 21, 2021 23:08
@charris
Copy link
Member

charris commented Sep 21, 2021

In case it is useful for writing tests, Fortran Lang uses fypp templates. I suspect you know that already :)

@charris charris merged commit 1372d26 into numpy:main Sep 22, 2021
@charris
Copy link
Member

charris commented Sep 22, 2021

Thanks @HaoZeke .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

f2py dead code elimination

4 participants