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

Move the pygmt.show_versions function to a separate file? #3276

Closed
seisman opened this issue May 29, 2024 · 3 comments · Fixed by #3277
Closed

Move the pygmt.show_versions function to a separate file? #3276

seisman opened this issue May 29, 2024 · 3 comments · Fixed by #3277
Labels
discussions Need more discussion before taking further actions
Milestone

Comments

@seisman
Copy link
Member

seisman commented May 29, 2024

The "Style Checks" workflow fails with ruff 0.4.6 because the show_versions() function has too many branches (https://github.com/GenericMappingTools/pygmt/actions/runs/9277819313/job/25527714073):

ruff check pygmt doc/conf.py examples
pygmt/__init__.py:80:5: PLR0915 Too many statements (57 > 50)
   |
80 | def show_versions(file=sys.stdout):
   |     ^^^^^^^^^^^^^ PLR09[15](https://github.com/GenericMappingTools/pygmt/actions/runs/9277819313/job/25527714073#step:5:16)
81 |     """
82 |     Print various dependency versions which are useful when submitting bug reports.
   |

Found 1 error.
make: *** [Makefile:66: check] Error 1
Error: Process completed with exit code 2.

The show_versions() function is also long with ~100 lines. Perhaps we should move it out of the pygmt/__init__.py file. Maybe pygmt/show_versions.py?

def show_versions(file=sys.stdout):

@seisman seisman added the discussions Need more discussion before taking further actions label May 29, 2024
@weiji14
Copy link
Member

weiji14 commented May 29, 2024

Oops, didn't see this when I opened #3277. I managed to fit things under 50 statements to fix the lint error, but we could move the show_versions() function if needed?

@seisman
Copy link
Member Author

seisman commented May 29, 2024

I prefer to move the show_versions function to:

  1. Keep the pygmt/__init__.py file shorter
  2. Move the private functions like _get_ghostscript_version out of the show_versions function so that we can better test them.

@weiji14
Copy link
Member

weiji14 commented May 29, 2024

Ok, I can move the function. Looking around, maybe we can put it under _show_versions.py following https://github.com/corteva/rioxarray/blob/0.15.5/rioxarray/_show_versions.py and https://github.com/scikit-learn/scikit-learn/blob/1.5.0/sklearn/utils/_show_versions.py? Other libraries like xarray puts it under util/print_versions.py, and pandas puts it under utils/_print_versions.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussions Need more discussion before taking further actions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants