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

Use reviewdog to annotate PR's with black and flake8 errors. #1424

Merged
merged 33 commits into from
Apr 16, 2024
Merged

Conversation

jcapriot
Copy link
Member

@jcapriot jcapriot commented Apr 15, 2024

Summary

Uses reviewdog to annotate PR's with suggested changes from black, and comment on errors from flake8

PR Checklist

  • If this is a work in progress PR, set as a Draft PR
  • Linted my code according to the style guides.
  • Added tests to verify changes to the code.
  • Added necessary documentation to any new functions/classes following the
    expect style.
  • Marked as ready for review (if this is was a draft PR), and converted
    to a Pull Request
  • Tagged @simpeg/simpeg-developers when ready for review.

What does this implement/fix?

Allows for automated reviewing of PR's from reviewdog detailing the flake8 and black errors when they fail, making it much more obvious to people who have submitted PR's about why those tests are failing.

@jcapriot jcapriot changed the title Create pull_request.yml Use reviewdog to annotate PR's with black and flake8 errors. Apr 15, 2024
SimPEG/data_misfit.py Outdated Show resolved Hide resolved
SimPEG/data.py Outdated Show resolved Hide resolved
@jcapriot
Copy link
Member Author

@santisoler Check this out!

@jcapriot
Copy link
Member Author

Obviously I've currently added purposeful places for the checks to fail to look at the output, I'll make sure it triggers azure tests as well after the "failures" are fixed.

@santisoler
Copy link
Member

Looks great! I think it could greatly improve the experience for new contributors.

Is there a way to get the github bot to use the reviewdog logo?? Would love to have it barking at us for forgetting to run black haha

@jcapriot
Copy link
Member Author

Looks great! I think it could greatly improve the experience for new contributors.

Is there a way to get the github bot to use the reviewdog logo?? Would love to have it barking at us for forgetting to run black haha

I don't see an option to get that setup through the pre-made actions for flake8 and black unfortunately.

@jcapriot
Copy link
Member Author

Perfect, looks to have properly triggered the azure pipelines runs after the style checks.

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.54%. Comparing base (fa5d794) to head (63f02fe).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1424      +/-   ##
==========================================
+ Coverage   74.16%   82.54%   +8.38%     
==========================================
  Files         167      167              
  Lines       25700    25700              
==========================================
+ Hits        19060    21214    +2154     
+ Misses       6640     4486    -2154     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jcapriot
Copy link
Member Author

Now just need to make sure it triggers a run on this branch…

SimPEG/data.py Outdated Show resolved Hide resolved
SimPEG/data.py Outdated Show resolved Hide resolved
@jcapriot
Copy link
Member Author

Ok, I'm happy with how this is setup now.
It does re-run the black/flake8 tests on azure, mostly because we still only want the tests to run if those things pass. Essentially the GitHub-actions workflow is only used to post review comments detailing for users about the specific flake8 error, or how to change the formatting to satisfy black.

@jcapriot jcapriot merged commit 999d18c into main Apr 16, 2024
18 checks passed
@jcapriot jcapriot deleted the reviewdog-1 branch April 16, 2024 22:24
thibaut-kobold added a commit to KoBoldMetals/simpeg that referenced this pull request May 8, 2024
…e_simpeg_021

* commit 'fe444e2d714ab1a3f1541575a9f8c4ad1e74894b': (73 commits)
  Fix distance calculation in `convert_survey_3d_to_2d_lines` (simpeg#1390)
  Rename spontaneous potential to self potential (simpeg#1422)
  Lowercase simpeg for generating coverage on Azure (simpeg#1443)
  Replace SimPEG for simpeg across docstrings (simpeg#1438)
  Update copyright year in __init__.py (simpeg#1436)
  Move to a PEP8 compliant package name. (simpeg#1430)
  Fix rst syntax in release notes for v0.21.0 (simpeg#1434)
  Replace use of `refine_tree_xyz` in DCIP tutorials (simpeg#1381)
  Add new Issue template for making a release (simpeg#1410)
  Safely run reviewdog on `pull_request_target` events (simpeg#1427)
  Use reviewdog to annotate PR's with black and flake8 errors. (simpeg#1424)
  Remove the parameters argument from docstring (simpeg#1417)
  Add release notes for v0.21.1 (simpeg#1416)
  Fix hard dask dependency (simpeg#1415)
  Publish documentation on azure (simpeg#1412)
  Add release notes for SimPEG v0.21 (simpeg#1409)
  Bump Black version to 24.3.0 (simpeg#1403)
  Remove link to "twitter" (simpeg#1406)
  Add Ying and Williams to AUTHORS.rst (simpeg#1405)
  Dask MetaSim (simpeg#1199)
  ...

# Conflicts:
#	simpeg/electromagnetics/base_1d_stitched.py
#	simpeg/electromagnetics/frequency_domain/simulation_1d_stitched.py
#	simpeg/electromagnetics/time_domain/receivers.py
#	simpeg/electromagnetics/time_domain/simulation_1d_stitched.py
#	simpeg/electromagnetics/utils/em1d_utils.py
#	simpeg/potential_fields/magnetics/_numba_functions.py
#	simpeg/regularization/laterally_constrained.py
#	simpeg/regularization/regularization_mesh_lateral.py
#	simpeg/regularization/rotated.py
#	simpeg/utils/__init__.py
#	simpeg/utils/model_utils.py
#	tests/base/regularizations/test_full_gradient.py
#	tests/base/regularizations/test_regularization.py
#	tests/base/test_mass_matrices.py
#	tests/base/test_model_utils.py
#	tests/base/test_utils.py
#	tests/em/fdem/inverse/adjoint/test_FDEM_adjointEB.py
#	tests/em/fdem/inverse/derivs/test_FDEM_derivs.py
#	tests/pf/test_forward_Mag_Linear.py
thibaut-kobold added a commit to KoBoldMetals/simpeg that referenced this pull request May 8, 2024
…al_update_21_release

* commit '07aeb65de865cd9df55e2d51ca840ea9d60f2234': (67 commits)
  Fix rst syntax in release notes for v0.21.0 (simpeg#1434)
  Replace use of `refine_tree_xyz` in DCIP tutorials (simpeg#1381)
  Add new Issue template for making a release (simpeg#1410)
  Safely run reviewdog on `pull_request_target` events (simpeg#1427)
  Use reviewdog to annotate PR's with black and flake8 errors. (simpeg#1424)
  Remove the parameters argument from docstring (simpeg#1417)
  Add release notes for v0.21.1 (simpeg#1416)
  Fix hard dask dependency (simpeg#1415)
  Publish documentation on azure (simpeg#1412)
  Add release notes for SimPEG v0.21 (simpeg#1409)
  Bump Black version to 24.3.0 (simpeg#1403)
  Remove link to "twitter" (simpeg#1406)
  Add Ying and Williams to AUTHORS.rst (simpeg#1405)
  Dask MetaSim (simpeg#1199)
  Update year in LICENSE (simpeg#1404)
  Update AUTHORS.rst (simpeg#1259)
  Minor adjustments to Sphinx configuration (simpeg#1398)
  Enforce regularization `weights` as dictionaries (simpeg#1344)
  Improve documentation for base simulation classes (simpeg#1295)
  Add link to User Tutorials to navbar in docs (simpeg#1401)
  ...

# Conflicts:
#	SimPEG/electromagnetics/time_domain/receivers.py
#	SimPEG/utils/__init__.py
#	SimPEG/utils/model_utils.py
#	tests/base/regularizations/test_full_gradient.py
#	tests/base/regularizations/test_regularization.py
#	tests/base/test_model_utils.py
#	tests/base/test_utils.py
#	tests/pf/test_forward_Mag_Linear.py
thibaut-kobold added a commit to thibaut-kobold/simpeg that referenced this pull request May 9, 2024
…ting_strategy

* commit 'fe444e2d714ab1a3f1541575a9f8c4ad1e74894b': (86 commits)
  Fix distance calculation in `convert_survey_3d_to_2d_lines` (simpeg#1390)
  Rename spontaneous potential to self potential (simpeg#1422)
  Lowercase simpeg for generating coverage on Azure (simpeg#1443)
  Replace SimPEG for simpeg across docstrings (simpeg#1438)
  Update copyright year in __init__.py (simpeg#1436)
  Move to a PEP8 compliant package name. (simpeg#1430)
  Fix rst syntax in release notes for v0.21.0 (simpeg#1434)
  Replace use of `refine_tree_xyz` in DCIP tutorials (simpeg#1381)
  Add new Issue template for making a release (simpeg#1410)
  Safely run reviewdog on `pull_request_target` events (simpeg#1427)
  Use reviewdog to annotate PR's with black and flake8 errors. (simpeg#1424)
  Remove the parameters argument from docstring (simpeg#1417)
  Add release notes for v0.21.1 (simpeg#1416)
  Fix hard dask dependency (simpeg#1415)
  Publish documentation on azure (simpeg#1412)
  Add release notes for SimPEG v0.21 (simpeg#1409)
  Bump Black version to 24.3.0 (simpeg#1403)
  Remove link to "twitter" (simpeg#1406)
  Add Ying and Williams to AUTHORS.rst (simpeg#1405)
  Dask MetaSim (simpeg#1199)
  ...

# Conflicts:
#	simpeg/utils/__init__.py
#	simpeg/utils/model_utils.py
#	tests/base/test_model_utils.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants