Skip to content

Commit

Permalink
Apply @jherland's suggestions from PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
mknorps committed Apr 18, 2023
1 parent 8e4c245 commit cf2fa8f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 22 deletions.
2 changes: 1 addition & 1 deletion fawltydeps/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def resolved_deps(self) -> Dict[str, Package]:
"""The resolved mapping of dependency names to provided import names."""
return resolve_dependencies(
(dep.name for dep in self.declared_deps),
custom_mapping_file=self.settings.custom_mapping_file,
custom_mapping_files=self.settings.custom_mapping_file,
custom_mapping=self.settings.custom_mapping,
pyenv_path=self.settings.pyenv,
install_deps=self.settings.install_deps,
Expand Down
38 changes: 18 additions & 20 deletions fawltydeps/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,19 @@ def __init__(
self._packages: Optional[Dict[str, Package]] = None

@staticmethod
def add_mapping_dicts(
mapping1: CustomMapping, mapping2: CustomMapping
) -> CustomMapping:
"""Add mapping dictionaries and normalise key (package) names."""
def accumulate_mappings(custom_mappings: Iterable[CustomMapping]) -> CustomMapping:
"""Merge mapping dictionaries and normalise key (package) names."""
result: CustomMapping = {}
for key, value in chain(mapping1.items(), mapping2.items()):
normalised_name = Package.normalize_name(key)
for name, imports in chain.from_iterable(cm.items() for cm in custom_mappings):
normalised_name = Package.normalize_name(name)
if normalised_name in result:
logger.info(
"Mapping for %s already found. Import names "
"from the second mapping are appended to ones "
"found in the first mapping.",
normalised_name,
)
result[normalised_name].extend(value)
else:
result[normalised_name] = value
result.setdefault(normalised_name, []).extend(imports)
return result

@property
Expand All @@ -157,32 +153,34 @@ def packages(self) -> Dict[str, Package]:
"""Gather a custom mapping given by a user.
Mapping may come from two sources:
* _mapping_paths_ which is a set of files in a given path, which is parsed to a ditionary
* _custom_mapping_ which is a dictionary of package to imports mapping.
* custom_mapping: an already-parsed CustomMapping, i.e. a dict mapping
package names to imports.
* mapping_paths: set of file paths, where each file contains a TOML-
formatted CustomMapping.
This enumerates the available packages _once_, and caches the result for
the remainder of this object's life in _packages.
"""
custom_mapping_from_files: Dict[str, List[str]] = {}

if self.mapping_paths is not None:
logger.debug(f"Loading user-defined mapping from {self.mapping_paths}")
for path in self.mapping_paths:
logger.debug(f"Loading user-defined mapping from {path}")
with open(path, "rb") as mapping_file:
custom_mapping_from_files = self.add_mapping_dicts(
custom_mapping_from_files, tomllib.load(mapping_file)
custom_mapping_from_files = self.accumulate_mappings(
[custom_mapping_from_files, tomllib.load(mapping_file)]
)

custom_mapping = custom_mapping_from_files

if self.custom_mapping is not None:
logger.debug("Applying user-defined mapping from settings.")

custom_mapping = self.add_mapping_dicts(
self.custom_mapping, custom_mapping_from_files
custom_mapping = self.accumulate_mappings(
[self.custom_mapping, custom_mapping_from_files]
)

# package name is normalised at this stage, `add_mapping_dicts` was called on
# package name is normalised at this stage, `accumulate_mappings` was called on
# input files and mapping from the configuration
return {
name: Package(
Expand Down Expand Up @@ -367,7 +365,7 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:

def resolve_dependencies(
dep_names: Iterable[str],
custom_mapping_file: Optional[Set[Path]] = None,
custom_mapping_files: Optional[Set[Path]] = None,
custom_mapping: Optional[CustomMapping] = None,
pyenv_path: Optional[Path] = None,
install_deps: bool = False,
Expand All @@ -388,10 +386,10 @@ def resolve_dependencies(
# each resolver in order until one of them returns a Package object. At
# that point we are happy, and don't consult any of the later resolvers.
resolvers: List[BasePackageResolver] = []
if custom_mapping_file or custom_mapping:
if custom_mapping_files or custom_mapping:
resolvers.append(
UserDefinedMapping(
mapping_paths=custom_mapping_file, custom_mapping=custom_mapping
mapping_paths=custom_mapping_files, custom_mapping=custom_mapping
)
)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def test_resolve_dependencies__focus_on_mappings(
assert (
resolve_dependencies(
dep_names,
custom_mapping_file=custom_mapping_files,
custom_mapping_files=custom_mapping_files,
custom_mapping=custom_mapping,
)
== expected
Expand Down

0 comments on commit cf2fa8f

Please sign in to comment.