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

Enhance visibility of missing dependencies #2931

Merged
merged 6 commits into from
Oct 17, 2024

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Oct 16, 2024

Curently PyYAML hides import errors in instantiated classes, and these are only logged at DEBUG level. This PR changes the log level to ERROR for these cases.

  • Tests added

@pnuu pnuu added enhancement code enhancements, features, improvements component:readers labels Oct 16, 2024
@pnuu pnuu self-assigned this Oct 16, 2024
@pnuu
Copy link
Member Author

pnuu commented Oct 16, 2024

This was the original log message when defusedxml was not installed for avhrr_l1b_eps reader. Note that the crucial info - missing defusedxml library - is reported in a DEBUG level message.

[DEBUG: 2024-10-16 11:01:28 : satpy.readers.yaml_reader] Reading ('/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/etc/readers/avhrr_l1b_eps.yaml',)
[INFO: 2024-10-16 11:01:28 : satpy.readers] Cannot use ['/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/etc/readers/avhrr_l1b_eps.yaml']
[DEBUG: 2024-10-16 11:01:28 : satpy.readers] while constructing a Python object
cannot find module 'satpy.readers.eps_l1b' (No module named 'defusedxml')
  in "/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/etc/readers/avhrr_l1b_eps.yaml", line 144, column 22
[WARNING: 2024-10-16 11:01:28 : satpy.readers] Don't know how to open the following files: {'/home/lahtinep/data/satellite/polar/avhrr_l1b_nat/AVHR_xxx_1B_M01_20241015100703Z_20241015114603Z_N_O_20241015105547Z.nat'}
Traceback (most recent call last):
  File "/home/lahtinep/bin/test_avhrr_l1b_nat.py", line 16, in <module>
    main()
  File "/home/lahtinep/bin/test_avhrr_l1b_nat.py", line 12, in main
    scn = Scene(reader="avhrr_l1b_eps", filenames=[fname])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/scene.py", line 155, in __init__
    self._readers = self._create_reader_instances(filenames=filenames,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/scene.py", line 176, in _create_reader_instances
    return load_readers(filenames=filenames,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/readers/__init__.py", line 590, in load_readers
    _check_reader_instances(reader_instances)
  File "/home/lahtinep/Software/pytroll/pytroll_packages/satpy/satpy/readers/__init__.py", line 629, in _check_reader_instances
    raise ValueError("No supported files found")
ValueError: No supported files found

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 93.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 96.08%. Comparing base (b8991c0) to head (3db6547).
Report is 112 commits behind head on main.

Files with missing lines Patch % Lines
satpy/readers/__init__.py 86.95% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2931    +/-   ##
========================================
  Coverage   96.07%   96.08%            
========================================
  Files         373      375     +2     
  Lines       54491    54612   +121     
========================================
+ Hits        52352    52472   +120     
- Misses       2139     2140     +1     
Flag Coverage Δ
behaviourtests 3.98% <6.66%> (-0.01%) ⬇️
unittests 96.17% <93.33%> (+<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.

@mraspaud
Copy link
Member

Is there a more fine-grained exception we could catch instead, for just building objects (which probably mean a missing import)?

@pnuu
Copy link
Member Author

pnuu commented Oct 16, 2024

I don't know. I'll have a look after figuring out the CodeScene complaint.

@coveralls
Copy link

coveralls commented Oct 16, 2024

Pull Request Test Coverage Report for Build 11366063321

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.

Details

  • 42 of 45 (93.33%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 96.177%

Changes Missing Coverage Covered Lines Changed/Added Lines %
satpy/readers/init.py 20 23 86.96%
Totals Coverage Status
Change from base Build 11324402105: 0.007%
Covered Lines: 52708
Relevant Lines: 54803

💛 - Coveralls

@pnuu
Copy link
Member Author

pnuu commented Oct 16, 2024

ca987bc should make CodeScene happier. Later today I'll look if there are more fine-grained failures we could check for, but I have a feeling there is none as YAMLError was there also originally.

@mraspaud
Copy link
Member

@pnuu
Copy link
Member Author

pnuu commented Oct 16, 2024

ConstructorError seems to work perfectly 👍

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, just a couple of comments/questions

satpy/readers/__init__.py Show resolved Hide resolved
satpy/readers/__init__.py Show resolved Hide resolved
@mraspaud mraspaud merged commit 5ee0729 into pytroll:main Oct 17, 2024
17 of 18 checks passed
@pnuu pnuu deleted the clarify-missing-reader-dependency-logging branch October 18, 2024 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants