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.

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

Mark unit tests with @pytest.mark.benchmark part 1 #2911

Merged
merged 55 commits into from
Dec 27, 2023
Merged

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Dec 25, 2023

Description of proposed changes

Follow-up to #2908, this is where we mark the unit tests for any PyGMT functions we want to benchmark with the @pytest.mark.benchmark decorator.

See #2911 (comment) below on which tests have been marked for benchmarking. Will first prioritize PyGMT functions that use temporary files, or are commonly used ones. General discussion on which tests to mark can go in #2910.

Note: We have purposely not installed pytest-mpl in benchmarks.yml, so the benchmarks are ran without any image comparison. This allows for faster benchmark runs, and it also means we don't need to setup dvc in the workflow.

Part 1 of addressing #2910. Covers test functions from test_geopandas to test_xyz2grd according to the list in #2910 (comment).

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Dec 25, 2023
@weiji14 weiji14 self-assigned this Dec 25, 2023
Copy link

codspeed-hq bot commented Dec 25, 2023

CodSpeed Performance Report

Merging #2911 will not alter performance

⚠️ No base runs were found

Falling back to comparing pytest/mark-benchmark (1c84a84) with main (b013f05)

Summary

✅ 1 untouched benchmarks

🆕 63 new benchmarks

Benchmarks breakdown

Benchmark main pytest/mark-benchmark Change
🆕 test_geopandas_info_geodataframe N/A 119 ms N/A
🆕 test_grdclip_no_outgrid N/A 86.5 ms N/A
🆕 test_grdcut_dataarray_in_dataarray_out N/A 86.2 ms N/A
🆕 test_grdcontour_labels N/A 99.1 ms N/A
🆕 test_grd2xyz N/A 43.6 ms N/A
🆕 test_grdfill_dataarray_out N/A 86.1 ms N/A
🆕 test_equalize_grid_no_outgrid N/A 86.3 ms N/A
🆕 test_grdfilter_dataarray_in_dataarray_out N/A 86.8 ms N/A
🆕 test_grdproject_no_outgrid[M10c] N/A 101.1 ms N/A
🆕 test_grdvolume_no_outgrid N/A 42.2 ms N/A
🆕 test_grdlandmask_no_outgrid N/A 2.2 s N/A
🆕 test_grdgradient_no_outgrid N/A 86.5 ms N/A
🆕 test_grdproject_no_outgrid[EPSG:3395 +width=10] N/A 104.2 ms N/A
🆕 test_info_pandas_dataframe_date_column[datetime64[ns]] N/A 57.5 ms N/A
🆕 test_histogram[Series] N/A 81.9 ms N/A
🆕 test_grdimage_grid_and_shading_with_xarray[png] N/A 5.9 s N/A
🆕 test_io_load_dataarray N/A 127.9 ms N/A
🆕 test_inset_aliases N/A 2.6 s N/A
🆕 test_legend_entries N/A 587.2 ms N/A
🆕 test_meca_spec_multiple_focalmecha[array2d] N/A 161.9 ms N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@seisman

This comment was marked as outdated.

@weiji14

This comment was marked as outdated.

@weiji14 weiji14 changed the title Mark unit tests with @pytest.mark.benchmark Mark unit tests with @pytest.mark.benchmark part 1 Dec 27, 2023
@weiji14 weiji14 marked this pull request as ready for review December 27, 2023 02:31
@weiji14
Copy link
Member Author

weiji14 commented Dec 27, 2023

Ok, have added @pytest.mark.benchmark to the second half of the list mentioned in #2910 (comment), except these ones:

  • test_helpers.py - too many random functions, unsure which ones need to be benchmarked
  • test_image.py - requires pulling test_logo.py from DVC
  • test_init.py - only tests pygmt.show_versions(), should we benchmark it since it's not really something that needs to have fast performance?
  • test_sphinx_gallery.py - requires sphinx_gallery to be installed, plus PyGMTScraper isn't really user facing code.

@weiji14 weiji14 added the skip-changelog Skip adding Pull Request to changelog label Dec 27, 2023
@weiji14 weiji14 added this to the 0.11.0 milestone Dec 27, 2023
@weiji14
Copy link
Member Author

weiji14 commented Dec 27, 2023

Ok, I'm seeing some slow benchmark tests taking >1min because we didn't download the cache, and there are lots of PytestReturnNotNoneWarnings because pytest-mpl isn't installed (see pytest-dev/pytest#10427). Gonna merge this first though and improve the benchmarks.yml CI workflow in a separate PR.

@seisman
Copy link
Member

seisman commented Dec 27, 2023

  • test_helpers.py - too many random functions, unsure which ones need to be benchmarked

None of them are important, so no need to benchmark now.

  • test_image.py - requires pulling test_logo.py from DVC

Yes, it's very annoying. In the ci_tests_legacy.yaml workflow, we also have to setup dvc due to this test. Perhaps we should use an image file in the GMT cache instead.

  • test_init.py - only tests pygmt.show_versions(), should we benchmark it since it's not really something that needs to have fast performance?

OK to ignore it.

  • test_sphinx_gallery.py - requires sphinx_gallery to be installed, plus PyGMTScraper isn't really user facing code.

OK to ignore it.

@weiji14 weiji14 merged commit c550f83 into main Dec 27, 2023
16 of 19 checks passed
@weiji14 weiji14 deleted the pytest/mark-benchmark branch December 27, 2023 02:59
@weiji14
Copy link
Member Author

weiji14 commented Dec 27, 2023

  • test_image.py - requires pulling test_logo.py from DVC

Yes, it's very annoying. In the ci_tests_legacy.yaml workflow, we also have to setup dvc due to this test. Perhaps we should use an image file in the GMT cache instead.

Yeah, it seems like we can use a remote file, e.g. fig.image("@circuit.png", ...). Let me open a PR - see #2922.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants