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.

Sign up for GitHub

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

Jump to bottom

Add type annotations #1761

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add type annotations #1761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 17 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[tool.mypy]
files = ["src/rez/", "src/rezplugins/"]
exclude = [
'.*/rez/data/.*',
'.*/rez/vendor/.*',
'.*/rez/tests/.*',
'.*/rez/utils/lint_helper.py',
]
disable_error_code = ["var-annotated", "import-not-found"]
check_untyped_defs = true
# allow this for now:
allow_redefinition = true

[[tool.mypy.overrides]]
module = 'rez.utils.lint_helper'
follow_imports = "skip"
3 changes: 0 additions & 3 deletions src/rez/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ def callback(sig, frame):
txt = ''.join(traceback.format_stack(frame))
print()
print(txt)
else:
callback = None

if callback:
signal.signal(signal.SIGUSR1, callback) # Register handler


Expand Down
70 changes: 46 additions & 24 deletions src/rez/build_process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# Copyright Contributors to the Rez Project


from __future__ import annotations

from rez.packages import iter_packages
from rez.exceptions import BuildProcessError, BuildContextResolveError, \
ReleaseHookCancellingError, RezError, ReleaseError, BuildError, \
Expand All @@ -18,6 +20,13 @@
import getpass
import os.path
import sys
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from rez.build_system import BuildSystem
from rez.packages import Package, Variant
from rez.release_vcs import ReleaseVCS
from rez.developer_package import DeveloperPackage


debug_print = config.debug_printer("package_release")
Expand All @@ -29,9 +38,16 @@ def get_build_process_types():
return plugin_manager.get_plugins('build_process')


def create_build_process(process_type, working_dir, build_system, package=None,
vcs=None, ensure_latest=True, skip_repo_errors=False,
ignore_existing_tag=False, verbose=False, quiet=False):
def create_build_process(process_type: str,
working_dir: str,
build_system: BuildSystem,
package=None,
vcs: ReleaseVCS | None = None,
ensure_latest: bool = True,
skip_repo_errors: bool = False,
ignore_existing_tag: bool = False,
verbose: bool = False,
quiet: bool = False) -> BuildProcess:
"""Create a :class:`BuildProcess` instance.

.. warning::
Expand All @@ -44,7 +60,7 @@ def create_build_process(process_type, working_dir, build_system, package=None,
if process_type not in process_types:
raise BuildProcessError("Unknown build process: %r" % process_type)

cls = plugin_manager.get_plugin_class('build_process', process_type)
cls = plugin_manager.get_plugin_class('build_process', process_type, BuildProcess)

return cls(working_dir, # ignored (deprecated)
build_system,
Expand Down Expand Up @@ -77,7 +93,8 @@ class BuildProcess(object):
def name(cls):
raise NotImplementedError

def __init__(self, working_dir, build_system, package=None, vcs=None,
def __init__(self, working_dir: str, build_system: BuildSystem, package=None,
vcs: ReleaseVCS | None = None,
ensure_latest=True, skip_repo_errors=False,
ignore_existing_tag=False, verbose=False, quiet=False):
"""Create a BuildProcess.
Expand Down Expand Up @@ -119,14 +136,15 @@ def __init__(self, working_dir, build_system, package=None, vcs=None,
self.package.config.build_directory)

@property
def package(self):
def package(self) -> DeveloperPackage:
return self.build_system.package

@property
def working_dir(self):
def working_dir(self) -> str:
return self.build_system.working_dir

def build(self, install_path=None, clean=False, install=False, variants=None):
def build(self, install_path: str | None = None, clean: bool = False,
Copy link
Member

Choose a reason for hiding this comment

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

Using str | None means we need to drop support for python 3.7. I'm not sure we are ready for this yet.

Copy link
Contributor Author

@chadrik chadrik Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, use of str | None is safe in python 3.7 as long as you use from __future__ import annotations. This backports behavior from python 3.9 that ensures that type annotations are recorded as strings within __annotations__ attributes, which means they are not evaluated at runtime unless inspect.get_annoations is called. The effect of from __future__ import annotations is that you can put basically anything you want into an annotation, it doesn't need to be valid at runtime.

The only thing breaking python 3.7 compatibility here is the use of TypedDict and Protocol, as mentioned in another comment. I presented 3 options for preserving the use of these classes in the other comment.

I noticed that the only python 3.7 tests that are currently run are for MacOS, which I took as an indicator that python 3.7 would be dropped soon. Is there a schedule for deprecation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, I fixed the python 3.7 compatibility issue with TypedDict and Protocol, so that should not be a blocker anymore.

install: bool = False, variants: list[int] | None = None) -> int:
"""Perform the build process.

Iterates over the package's variants, resolves the environment for
Expand All @@ -149,7 +167,8 @@ def build(self, install_path=None, clean=False, install=False, variants=None):
"""
raise NotImplementedError

def release(self, release_message=None, variants=None):
def release(self, release_message: str | None = None,
variants: list[int] | None = None) -> int:
"""Perform the release process.

Iterates over the package's variants, building and installing each into
Expand All @@ -167,7 +186,7 @@ def release(self, release_message=None, variants=None):
"""
raise NotImplementedError

def get_changelog(self):
def get_changelog(self) -> str | None:
"""Get the changelog since last package release.

Returns:
Expand All @@ -187,7 +206,8 @@ def repo_operation(self):
except exc_type as e:
print_warning("THE FOLLOWING ERROR WAS SKIPPED:\n%s" % str(e))

def visit_variants(self, func, variants=None, **kwargs):
def visit_variants(self, func, variants: list[int] | None = None,
**kwargs) -> tuple[int, list[str | None]]:
"""Iterate over variants and call a function on each."""
if variants:
present_variants = range(self.package.num_variants)
Expand Down Expand Up @@ -215,7 +235,7 @@ def visit_variants(self, func, variants=None, **kwargs):

return num_visited, results

def get_package_install_path(self, path):
def get_package_install_path(self, path: str) -> str:
"""Return the installation path for a package (where its payload goes).

Args:
Expand All @@ -230,7 +250,8 @@ def get_package_install_path(self, path):
package_version=self.package.version
)

def create_build_context(self, variant, build_type, build_path):
def create_build_context(self, variant: Variant, build_type: BuildType,
build_path: str) -> tuple[ResolvedContext, str]:
"""Create a context to build the variant within."""
request = variant.get_requires(build_requires=True,
private_build_requires=True)
Expand Down Expand Up @@ -274,7 +295,7 @@ def create_build_context(self, variant, build_type, build_path):
raise BuildContextResolveError(context)
return context, rxt_filepath

def pre_release(self):
def pre_release(self) -> None:
release_settings = self.package.config.plugins.release_vcs

# test that the release path exists
Expand Down Expand Up @@ -322,7 +343,7 @@ def pre_release(self):
else:
break

def post_release(self, release_message=None):
def post_release(self, release_message=None) -> None:
tag_name = self.get_current_tag_name()

if self.vcs is None:
Expand All @@ -332,7 +353,7 @@ def post_release(self, release_message=None):
with self.repo_operation():
self.vcs.create_release_tag(tag_name=tag_name, message=release_message)

def get_current_tag_name(self):
def get_current_tag_name(self) -> str:
release_settings = self.package.config.plugins.release_vcs
try:
tag_name = self.package.format(release_settings.tag_name)
Expand All @@ -342,7 +363,7 @@ def get_current_tag_name(self):
tag_name = "unversioned"
return tag_name

def run_hooks(self, hook_event, **kwargs):
def run_hooks(self, hook_event, **kwargs) -> None:
hook_names = self.package.config.release_hooks or []
hooks = create_release_hooks(hook_names, self.working_dir)

Expand All @@ -357,12 +378,12 @@ def run_hooks(self, hook_event, **kwargs):
"%s cancelled by %s hook '%s': %s:\n%s"
% (hook_event.noun, hook_event.label, hook.name(),
e.__class__.__name__, str(e)))
except RezError:
except RezError as e:
debug_print("Error in %s hook '%s': %s:\n%s"
% (hook_event.label, hook.name(),
e.__class__.__name__, str(e)))

def get_previous_release(self):
def get_previous_release(self) -> Package | None:
release_path = self.package.config.release_packages_path
it = iter_packages(self.package.name, paths=[release_path])
packages = sorted(it, key=lambda x: x.version, reverse=True)
Expand All @@ -372,18 +393,19 @@ def get_previous_release(self):
return package
return None

def get_changelog(self):
def get_changelog(self) -> str | None:
previous_package = self.get_previous_release()
if previous_package:
previous_revision = previous_package.revision
else:
previous_revision = None

changelog = None
with self.repo_operation():
changelog = self.vcs.get_changelog(
previous_revision,
max_revisions=config.max_package_changelog_revisions)
if self.vcs:
with self.repo_operation():
changelog = self.vcs.get_changelog(
previous_revision,
max_revisions=config.max_package_changelog_revisions)

return changelog

Expand Down
Loading
Loading