Update check_satpy to use new show_version to display package versions

[verduijn]

MR to add show_versions to satpy.utils to allow for use in satpy.utils.check_satpy.
Use case is to help in figuring out run-time environment factors while debugging issues.

[djhoese]

I don't know how I feel about these changes and I haven't felt great about the discussion since it was restarted in the related issue. This may mean that I'm just missing something from the feature request, but overall it feels like a lot of flexibility is being added and things changed for something that I consider rather static and single-use.

1. Why does show_versions need to have a packages keyword argument? In my opinion (others can feel free to disagree) this should just be a hardcoded list. This function is for us, the maintainers, not for users. Users run it to tell us what we need to know.
2. Why should check_satpy call show_versions? It feels like they should be separate. There are times when duplicated code is acceptable and if these changes were done to reduce duplication I'm not sure it is worth it.
3. Why did extras need to be renamed?

It seems like both check_satpy and show_versions were implemented/re-implemented to be even more flexible and to be a generic "check this import for me" and I'm just not sure that task is complicated enough that Satpy needs to be providing a generic utility for users to use. What I tried to say above is that I think of these types of functions as tools to assist maintainers in helping users, not as generic tools for the user.

[verduijn]

Thanks for the feedback!

I figured that check_satpy was in the default issue template, so it could be useful to have the versions information in there.

From there:

• I thought i could expose the show_versions functionality itself without much downside
• check_satpy was flexible with extras so I just did the same
• since extras and packages was essentially the same I just renamed it

rounding up I think we should either:

• remove show_versions and add it, with hardcoded packages, in check_satpy
• Having a hard coded show_versions function and reverting the check_satpy function if it's not desirable to add the versions in check_satpy.

[djhoese]

Good point with it being in the issue template. We wouldn't want users to have to run multiple things to show similar information. In that case I agree that the versions after the readers/writers in check_satpy does make more sense than the "ok" that is there now. Ok I'm coming around. Thanks for explaining your reasoning.

[djhoese]

It's hard to tell from github's diff display, but is this function used anymore?

[verduijn]

good point! I've removed the now unused _check_imports.

[verduijn]

I guess it's ready for review?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.05%. Comparing base (9e3b342) to head (692c9b1).
Report is 358 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2913      +/-   ##
==========================================
- Coverage   96.06%   96.05%   -0.01%     
==========================================
  Files         373      373              
  Lines       54465    54479      +14     
==========================================
+ Hits        52321    52330       +9     
- Misses       2144     2149       +5     

Flag	Coverage Δ
behaviourtests	3.99% <11.90%> (+<0.01%)	⬆️
unittests	96.15% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[djhoese]

From a basic inspection, these changes seem good to me. I'd like some other reviews though. (@mraspaud @pnuu @sfinkens maybe?)

[coveralls]

Pull Request Test Coverage Report for Build 11236933264

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 42 of 42 (100.0%) changed or added relevant lines in 2 files are covered.
• 14 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-0.01%) to 96.151%

---

Files with Coverage Reduction	New Missed Lines	%
satpy/composites/config_loader.py	2	94.12%
satpy/tests/utils.py	2	93.16%
satpy/tests/reader_tests/gms/test_gms5_vissr_l1b.py	3	98.67%
satpy/tests/reader_tests/gms/test_gms5_vissr_navigation.py	3	97.18%
satpy/readers/msi_safe.py	4	98.41%

Totals
Change from base Build 10957325874:	-0.01%
Covered Lines:	52564
Relevant Lines:	54668

---

💛 - Coveralls

[pnuu]

Just couple of styling issues that I think linter should have caught.

[sfinkens]

Nice work, thanks! I have one wish for the tests.