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 mindeps generation logic to separate function #5

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
13 changes: 2 additions & 11 deletions resolver/mindeps/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
# SPDX-License-Identifier: MIT

import operator
from resolver.mindeps.__main__ import entrypoint, get_min_deps # noqa: F401

from typing import Iterable, Sequence

import resolver


class MinimumDependencyProvider(resolver.Provider):
def sort_candidates(
self,
candidates: Iterable[resolver.Candidate],
) -> Sequence[resolver.Candidate]:
return sorted(candidates, key=operator.attrgetter('version'), reverse=False)
__all__ = ('entrypoint', 'get_min_deps')
64 changes: 45 additions & 19 deletions resolver/mindeps/__main__.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# SPDX-License-Identifier: MIT

import argparse
import operator
import os
import os.path
import pathlib
import tempfile

from typing import Iterable, Sequence, Set
from typing import Dict, Iterable, List, Optional, Sequence, Set

import packaging.markers
import packaging.requirements
Expand Down Expand Up @@ -91,39 +92,43 @@ def main_parser() -> argparse.ArgumentParser:
return parser


def task() -> None: # noqa: C901
parser = main_parser()
args = parser.parse_args()
class MinimumDependencyProvider(resolver.Provider):
def sort_candidates(
self,
candidates: Iterable[resolver.Candidate],
) -> Sequence[resolver.Candidate]:
return sorted(candidates, key=operator.attrgetter('version'), reverse=False)
Comment on lines +95 to +100
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any particular reason you moved this here? Unless there's a good reason not to, I think we should keep it in resolver.mindeps. Also notice that removing it from there breaks imports, this class is meant to be available for people to use.


if args.requirements:
for bad_arg in _MARKER_KEYS + ('extras',):
if bad_arg in args:
resolver.__main__._error(f'Option --{bad_arg} not supported when specifying bare requirements')

reporter = resolver.__main__.VerboseReporter if args.verbose else resolvelib.BaseReporter
def get_min_deps(
requirements: List[str],
reporter: Optional[resolvelib.BaseReporter] = None,
extras: Optional[Set[str]] = None,
markers: Optional[Dict[str, str]] = None
) -> Dict[str, str]:
Comment on lines +103 to +108
Copy link
Owner

Choose a reason for hiding this comment

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

Let's try to avoid using specific data types unless necessary

Suggested change
def get_min_deps(
requirements: List[str],
reporter: Optional[resolvelib.BaseReporter] = None,
extras: Optional[Set[str]] = None,
markers: Optional[Dict[str, str]] = None
) -> Dict[str, str]:
def get_min_deps(
requirements: Collection[str],
reporter: Optional[resolvelib.BaseReporter] = None,
extras: Optional[Collection[str]] = None,
markers: Optional[Mapping[str, str]] = None
) -> Mapping[str, str]:

reporter = reporter or resolvelib.BaseReporter
Copy link
Owner

Choose a reason for hiding this comment

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

nitpick: I'd prefer to be more explicit here, even if it takes an extra line.

Suggested change
reporter = reporter or resolvelib.BaseReporter
if not reporter:
reporter = resolvelib.BaseReporter

package_resolver = resolvelib.Resolver(
resolver.mindeps.MinimumDependencyProvider(
MinimumDependencyProvider(
'/tmp/resolver-cache' if os.name == 'posix' else None
),
reporter(),
)

requirements: Iterable[packaging.requirements.Requirement] = map(
packaging.requirements.Requirement,
args.requirements or _project_requirements(),
packaging.requirements.Requirement, requirements
)

extras = set(vars(args).get('extras', {})) | {''}
if any(arg in _MARKER_KEYS for arg in vars(args)):
extras = extras.copy() | {''} if extras else {''}
Copy link
Owner

Choose a reason for hiding this comment

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

nitpick:

The .copy() call shouldn't be needed here because the | operator does not mutate the set, it creates a new one.

Since extras will always be set now, you could just do extras = extras | {''}. Note that we can't do extras |= {''} because |= does mutate the original set 😅

But anyway, since we are changing the type hint to Collection, we need to rewrite this snippet anyway.

if markers is not None and any(marker in _MARKER_KEYS for marker in markers):
Copy link
Owner

Choose a reason for hiding this comment

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

nitpick: I'd skip the is None, to simplify the condition.

Suggested change
if markers is not None and any(marker in _MARKER_KEYS for marker in markers):
if markers and any(marker in _MARKER_KEYS for marker in markers):

marker_env = {
k: v for k, v in vars(args).items()
k: v for k, v in markers.items()
if k in _MARKER_KEYS
}
else:
marker_env = packaging.markers.default_environment()

resolver_requirements: Set[packaging.requirements.Requirement] = set()
for requirement in requirements:
for requirement in list(requirements):
for extra in extras:
if not requirement.marker:
resolver_requirements.add(requirement)
Expand All @@ -133,13 +138,34 @@ def task() -> None: # noqa: C901

result = package_resolver.resolve(resolver_requirements)

if args.verbose:
print('\n--- Solution ---')

pinned = {
candidate.name: candidate.version
for candidate in result.mapping.values()
}

return pinned


def task() -> None: # noqa: C901
parser = main_parser()
args = parser.parse_args()

if args.requirements:
for bad_arg in _MARKER_KEYS + ('extras',):
if bad_arg in args:
resolver.__main__._error(f'Option --{bad_arg} not supported when specifying bare requirements')

reporter = resolver.__main__.VerboseReporter if args.verbose else resolvelib.BaseReporter

if args.verbose:
print('\n--- Solution ---')

pinned = get_min_deps(
args.requirements or _project_requirements(),
reporter=reporter,
extras=set(vars(args).get('extras', {})),
markers=vars(args)
)
for name, version in pinned.items():
print(f'{name}=={str(version)}')

Expand Down