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
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 34 additions & 15 deletions satpy/readers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python

Check notice on line 1 in satpy/readers/__init__.py

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 4.88 to 4.60, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
# -*- coding: utf-8 -*-
# Copyright (c) 2015-2018 Satpy developers
#
Expand Down Expand Up @@ -557,31 +557,24 @@
reader, filenames, remaining_filenames = _get_reader_and_filenames(reader, filenames)
(reader_kwargs, reader_kwargs_without_filter) = _get_reader_kwargs(reader, reader_kwargs)

for idx, reader_configs in enumerate(configs_for_reader(reader)):
if isinstance(filenames, dict):
readers_files = set(filenames[reader[idx]])
else:
readers_files = remaining_filenames
if reader_kwargs is None:
reader_kwargs = {}

Check warning on line 561 in satpy/readers/__init__.py

View check run for this annotation

Codecov / codecov/patch

satpy/readers/__init__.py#L561

Added line #L561 was not covered by tests
mraspaud marked this conversation as resolved.
Show resolved Hide resolved

try:
reader_instance = load_reader(
reader_configs,
**reader_kwargs[None if reader is None else reader[idx]])
except (KeyError, IOError, yaml.YAMLError) as err:
LOG.info("Cannot use %s", str(reader_configs))
LOG.debug(str(err))
for idx, reader_configs in enumerate(configs_for_reader(reader)):
readers_files = _get_readers_files(filenames, reader, idx, remaining_filenames)
reader_instance = _get_reader_instance(reader, reader_configs, idx, reader_kwargs)
if reader_instance is None or not readers_files:
# Reader initiliasation failed or no files were given
continue

if not readers_files:
# we weren't given any files for this reader
continue
loadables = reader_instance.select_files_from_pathnames(readers_files)
if loadables:
reader_instance.create_storage_items(
loadables,
fh_kwargs=reader_kwargs_without_filter[None if reader is None else reader[idx]])
reader_instances[reader_instance.name] = reader_instance
remaining_filenames -= set(loadables)

Check notice on line 577 in satpy/readers/__init__.py

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Method

load_readers is no longer above the threshold for cyclomatic complexity. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
if not remaining_filenames:
break

Expand All @@ -590,6 +583,32 @@
return reader_instances


def _get_readers_files(filenames, reader, idx, remaining_filenames):
if isinstance(filenames, dict):
return set(filenames[reader[idx]])
return remaining_filenames


def _get_reader_instance(reader, reader_configs, idx, reader_kwargs):
reader_instance = None
try:
reader_instance = load_reader(
reader_configs,
**reader_kwargs[None if reader is None else reader[idx]])
except (KeyError, IOError) as err:
mraspaud marked this conversation as resolved.
Show resolved Hide resolved
LOG.info("Cannot use %s", str(reader_configs))
LOG.debug(str(err))

Check warning on line 600 in satpy/readers/__init__.py

View check run for this annotation

Codecov / codecov/patch

satpy/readers/__init__.py#L599-L600

Added lines #L599 - L600 were not covered by tests
except yaml.constructor.ConstructorError as err:
_log_yaml_error(reader_configs, err)

return reader_instance


def _log_yaml_error(reader_configs, err):
LOG.error("Problem with %s", str(reader_configs))
LOG.error(str(err))


def _early_exit(filenames, reader):
if not filenames and not reader:
# used for an empty Scene
Expand Down
23 changes: 23 additions & 0 deletions satpy/tests/test_readers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python

Check notice on line 1 in satpy/tests/test_readers.py

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Lines of Code in a Single File

The lines of code increases from 848 to 863, improve code health by reducing it to 600. The number of Lines of Code in a single file. More Lines of Code lowers the code health.

Check notice on line 1 in satpy/tests/test_readers.py

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Number of Functions in a Single Module

The number of functions increases from 99 to 101, threshold = 75. This file contains too many functions. Beyond a certain threshold, more functions lower the code health.

Check notice on line 1 in satpy/tests/test_readers.py

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Low Cohesion

The number of different responsibilities increases from 52 to 53, threshold = 4. Cohesion is calculated using the LCOM4 metric. Low cohesion means that the module/class has multiple unrelated responsibilities, doing too many things and breaking the Single Responsibility Principle.
# -*- coding: utf-8 -*-
# Copyright (c) 2019, 2022, 2023 Satpy developers
#
Expand Down Expand Up @@ -258,6 +258,11 @@
Assumes that the VIIRS SDR reader exists and works.
"""

@pytest.fixture(autouse=True)
def inject_fixtures(self, caplog): # noqa: PT004
"""Inject caplog to the test class."""
self._caplog = caplog

def setUp(self):
"""Wrap HDF5 file handler with our own fake handler."""
from satpy.readers.viirs_sdr import VIIRSSDRFileHandler
Expand Down Expand Up @@ -439,6 +444,24 @@
assert "abi_l1b" in readers
assert len(list(readers["abi_l1b"].available_dataset_ids)) == 0

@mock.patch("satpy.readers.load_reader")
pnuu marked this conversation as resolved.
Show resolved Hide resolved
def test_yaml_error_message(self, load_reader):
"""Test that YAML errors are logged properly."""
import logging

import yaml

from satpy.readers import load_readers

filenames = ["AVHR_xxx_1B_M01_20241015100703Z_20241015114603Z_N_O_20241015105547Z.nat"]
error_message = "YAML test error message"
load_reader.side_effect = yaml.constructor.ConstructorError(error_message)

with self._caplog.at_level(logging.ERROR):
with pytest.raises(ValueError, match="No supported files found"):
_ = load_readers(filenames=filenames, reader="avhrr_l1b_eps")
assert error_message in self._caplog.text


class TestFindFilesAndReaders:
"""Test the find_files_and_readers utility function."""
Expand Down
Loading