Skip to content

TST: Initialize f2py2e tests of the F2PY CLI #20668

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
Merged
merged 4 commits into from
Apr 1, 2022
Merged

TST: Initialize f2py2e tests of the F2PY CLI #20668

merged 4 commits into from
Apr 1, 2022

Conversation

HaoZeke
Copy link
Member

@HaoZeke HaoZeke commented Dec 26, 2021

Essentially this branch: HaoZeke/f2py_skel#15

Increases F2PY coverage by around 15 percent. For the CLI itself it covers the major features (around 70 percent), with the exception of mostly numpy.distutils stuff.

More importantly, sets the groundwork for #20056, in that passing the same testsuite should indicate feature parity.

Does not depend on the refactors.

Changes:

  • Tests for most of the current CLI

@HaoZeke HaoZeke requested a review from melissawm December 26, 2021 20:26
@HaoZeke
Copy link
Member Author

HaoZeke commented Dec 26, 2021 •
edited
Loading

I tested all the pytest capture mechanisms and nothing other than --capture=tee-sys would do for capturing the stderr stream f2py writes to.

Setting up special handling for f2py tests in runpython.py seemed clunky so adding to the global set seemed like the best move. Does not change any existing tests (other than to make the f2py ones a bit more verbose).

@HaoZeke HaoZeke force-pushed the f2py2eTests branch 5 times, most recently from bc5cb05 to 6fa453d Compare December 27, 2021 15:24
charris
charris reviewed Dec 28, 2021
View reviewed changes
numpy/f2py/tests/src/cli/hi77.f
@@ -0,0 +1,3 @@
SUBROUTINE HI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could probably use a renaming to hello_world77.f or some such.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the other be renamed similarly? We currently have hi77.f and hiworld.f90. It'd be good to have a template name format and redo all of them at once, all the test sources are oddly named.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the test sources are oddly named.

Having consistent names would be nice. It isn't a priority, but feel free to do it. It is currently hard to figure out what all the tests do. What ideas do you have for a "template name format"?

Copy link
Member Author

@HaoZeke HaoZeke Dec 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using the test name or after the refactor the submodule name so:

  • cli_hello_world.f77
  • cli_hello_world.f90
  • frontend_no_space (instead of gh15035.f)

Essentially from what we have, even submodule_test_name is an upgrade.
Perhaps in a different PR though?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, avoiding names like hi and blah seems like a good idea in general. Not a blocker though.

@charris
Copy link
Member

charris commented Dec 28, 2021

for capturing the stderr stream

Will those errors eventually be Python exceptions?

@charris
Copy link
Member

charris commented Dec 28, 2021

Still some errors.

@HaoZeke
Copy link
Member Author

HaoZeke commented Dec 30, 2021 •
edited
Loading

for capturing the stderr stream

Will those errors eventually be Python exceptions?

I don't think so. These are basically written to sys.stderr and sys.stdout during the wrapper generation, not during the execution of the module itself so I don't think it makes sense to have these be Python exceptions (in that we aren't really planning to handle them or anything).

However I have considered reworking the F2PY logging system with logging, which would clean all of this up a lot.

EDIT: Somehow one of the sys.stderr write-outs wasn't working (buffering issues?), but having fixed that (in a rather ham handed way, with print("blah", file=sys.stderr) this PR no longer adds to the numpy pytest.ini file.

@HaoZeke
Copy link
Member Author

HaoZeke commented Dec 30, 2021

I'm not sure I know what's wrong with the Windows tests, some (32-bit) seem to be related to not finding gfortran-5 and the others seem to have to do with different outputs which break the assertions (which definitely shouldn't happen).... Perhaps someone with more experience of the numpy Azure pipelines could shed some light on this?

@melissawm
Copy link
Member

I'm not sure I know what's wrong with the Windows tests, some (32-bit) seem to be related to not finding gfortran-5 and the others seem to have to do with different outputs which break the assertions (which definitely shouldn't happen).... Perhaps someone with more experience of the numpy Azure pipelines could shed some light on this?

Not sure about the 32-bit tests but running on windows confirms the problem with the slashes in the assert for outputs.

@HaoZeke HaoZeke mentioned this pull request Mar 12, 2022
Merged
3 tasks
@melissawm
Copy link
Member

This is looking good now, so let's try it - thanks @HaoZeke !

@melissawm melissawm merged commit 729ad4f into numpy:main Apr 1, 2022
melissawm pushed a commit to melissawm/numpy that referenced this pull request Apr 12, 2022
@HaoZeke @melissawm
TST: Initialize f2py2e tests of the F2PY CLI (numpy#20668)
03eeab0 
Increases F2PY coverage by around 15 percent. For the CLI itself it covers the major features (around 70 percent), with the exception of mostly numpy.distutils stuff.

More importantly, sets the groundwork for numpy#20056, in that passing the same testsuite should indicate feature parity.
@HaoZeke HaoZeke mentioned this pull request Nov 13, 2023
Merged
@HaoZeke HaoZeke deleted the f2py2eTests branch November 13, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charris charris charris left review comments

@melissawm melissawm Awaiting requested review from melissawm

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

Successfully merging this pull request may close these issues.
3 participants
@HaoZeke @charris @melissawm