diff --git a/.github/workflows/python-quality.yml b/.github/workflows/python-quality.yml index 1ab5714905..b185f78c74 100644 --- a/.github/workflows/python-quality.yml +++ b/.github/workflows/python-quality.yml @@ -18,15 +18,20 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - name: Set up Python - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - name: Install dependencies - run: | - pip install --upgrade pip - pip install .[dev] - - run: black --check tests src - - run: isort --check-only tests src - - run: flake8 tests src + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: | + pip install --upgrade pip + pip install .[dev] + - run: black --check tests src + - run: isort --check-only tests src + - run: flake8 tests src + - run: python utils/check_static_imports.py + + # Run type checking at least on huggingface_hub root file to check all modules + # that can be lazy-loaded actually exist. + - run: mypy src/huggingface_hub/__init__.py --follow-imports=silent diff --git a/Makefile b/Makefile index 97d3274367..c0d55320a6 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,19 @@ .PHONY: quality style test -check_dirs := tests src setup.py +check_dirs := tests src utils setup.py quality: black --check $(check_dirs) isort --check-only $(check_dirs) flake8 $(check_dirs) + python utils/check_static_imports.py style: black $(check_dirs) isort $(check_dirs) + python utils/check_static_imports.py --update-file test: HUGGINGFACE_CO_STAGING=1 pytest -sv ./tests/ diff --git a/setup.py b/setup.py index e310583370..15dbda7a98 100644 --- a/setup.py +++ b/setup.py @@ -36,18 +36,21 @@ def get_version() -> str: extras["tensorflow"] = ["tensorflow", "pydot", "graphviz"] extras["testing"] = [ + "datasets", + "isort>=5.5.4", + "jedi", + "Jinja2", "pytest", "pytest-cov", - "datasets", "soundfile", - "Jinja2", ] extras["quality"] = [ "black==22.3", - "isort>=5.5.4", "flake8>=3.8.3", "flake8-bugbear", + "isort>=5.5.4", + "mypy", ] extras["all"] = extras["testing"] + extras["quality"] diff --git a/src/huggingface_hub/__init__.py b/src/huggingface_hub/__init__.py index b6a641b9c6..a5c12f2b82 100644 --- a/src/huggingface_hub/__init__.py +++ b/src/huggingface_hub/__init__.py @@ -1,7 +1,3 @@ -# flake8: noqa -# There's no way to ignore "F401 '...' imported but unused" warnings in this -# module, but to preserve other warnings. So, don't check this module at all. - # Copyright 2020 The HuggingFace Team. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,18 +13,168 @@ # limitations under the License. # *********** -# vendored from https://github.com/scientific-python/lazy_loader +# `huggingface_hub` init has 2 modes: +# - Normal usage: +# If imported to use it, all modules and functions are lazy-loaded. This means +# they exist at top level in module but are imported only the first time they are +# used. This way, `from huggingface_hub import something` will import `something` +# quickly without the hassle of importing all the features from `huggingface_hub`. +# - Static check: +# If statically analyzed, all modules and functions are loaded normally. This way +# static typing check works properly as well as autocomplete in text editors and +# IDEs. +# +# The static model imports are done inside the `if TYPE_CHECKING:` statement at +# the bottom of this file. Since module/functions imports are duplicated, it is +# mandatory to make sure to add them twice when adding one. This is checked in the +# `make quality` command. +# +# To update the static imports, please run the following command and commit the changes. +# ``` +# # Use script +# python utils/check_static_imports.py --update-file +# +# # Or run style on codebase +# make style +# ``` +# +# *********** +# Lazy loader vendored from https://github.com/scientific-python/lazy_loader import importlib -import importlib.util -import inspect import os import sys -import types -import warnings +from typing import TYPE_CHECKING -class _LazyImportWarning(Warning): - pass +__version__ = "0.10.0.dev0" + +# Alphabetical order of definitions is ensured in tests +# WARNING: any comment added in this dictionary definition will be lost when +# re-generating the file ! +_SUBMOD_ATTRS = { + "_snapshot_download": [ + "snapshot_download", + ], + "commands.user": [ + "notebook_login", + ], + "community": [ + "Discussion", + "DiscussionComment", + "DiscussionCommit", + "DiscussionEvent", + "DiscussionStatusChange", + "DiscussionTitleChange", + "DiscussionWithDetails", + ], + "constants": [ + "CONFIG_NAME", + "FLAX_WEIGHTS_NAME", + "HUGGINGFACE_CO_URL_HOME", + "HUGGINGFACE_CO_URL_TEMPLATE", + "PYTORCH_WEIGHTS_NAME", + "REPO_TYPE_DATASET", + "REPO_TYPE_MODEL", + "REPO_TYPE_SPACE", + "TF2_WEIGHTS_NAME", + "TF_WEIGHTS_NAME", + ], + "fastai_utils": [ + "_save_pretrained_fastai", + "from_pretrained_fastai", + "push_to_hub_fastai", + ], + "file_download": [ + "cached_download", + "hf_hub_download", + "hf_hub_url", + "try_to_load_from_cache", + ], + "hf_api": [ + "CommitOperation", + "CommitOperationAdd", + "CommitOperationDelete", + "DatasetSearchArguments", + "HfApi", + "HfFolder", + "ModelSearchArguments", + "change_discussion_status", + "comment_discussion", + "create_commit", + "create_discussion", + "create_pull_request", + "create_repo", + "dataset_info", + "delete_file", + "delete_repo", + "edit_discussion_comment", + "get_dataset_tags", + "get_discussion_details", + "get_full_repo_name", + "get_model_tags", + "get_repo_discussions", + "list_datasets", + "list_metrics", + "list_models", + "list_repo_files", + "merge_pull_request", + "model_info", + "move_repo", + "rename_discussion", + "repo_type_and_id_from_hf_id", + "set_access_token", + "space_info", + "unset_access_token", + "update_repo_visibility", + "upload_file", + "upload_folder", + "whoami", + ], + "hub_mixin": [ + "ModelHubMixin", + "PyTorchModelHubMixin", + ], + "inference_api": [ + "InferenceApi", + ], + "keras_mixin": [ + "KerasModelHubMixin", + "from_pretrained_keras", + "push_to_hub_keras", + "save_pretrained_keras", + ], + "repocard": [ + "DatasetCard", + "ModelCard", + "metadata_eval_result", + "metadata_load", + "metadata_save", + "metadata_update", + ], + "repocard_data": [ + "CardData", + "DatasetCardData", + "EvalResult", + "ModelCardData", + ], + "repository": [ + "Repository", + ], + "utils": [ + "CachedFileInfo", + "CachedRepoInfo", + "CachedRevisionInfo", + "CorruptedCacheException", + "DeleteCacheStrategy", + "HFCacheInfo", + "logging", + "scan_cache_dir", + ], + "utils.endpoint_helpers": [ + "DatasetFilter", + "ModelFilter", + ], +} def _attach(package_name, submodules=None, submod_attrs=None): @@ -114,119 +260,111 @@ def __dir__(): return __getattr__, __dir__, list(__all__) -# ************ - -__version__ = "0.10.0.dev0" - - __getattr__, __dir__, __all__ = _attach( - __name__, - submodules=[], - submod_attrs={ - "commands.user": ["notebook_login"], - "constants": [ - "CONFIG_NAME", - "FLAX_WEIGHTS_NAME", - "HUGGINGFACE_CO_URL_HOME", - "HUGGINGFACE_CO_URL_TEMPLATE", - "PYTORCH_WEIGHTS_NAME", - "REPO_TYPE_DATASET", - "REPO_TYPE_MODEL", - "REPO_TYPE_SPACE", - "TF2_WEIGHTS_NAME", - "TF_WEIGHTS_NAME", - ], - "fastai_utils": [ - "_save_pretrained_fastai", - "from_pretrained_fastai", - "push_to_hub_fastai", - ], - "file_download": [ - "cached_download", - "hf_hub_download", - "hf_hub_url", - "try_to_load_from_cache", - ], - "hf_api": [ - "CommitOperation", - "CommitOperationAdd", - "CommitOperationDelete", - "DatasetSearchArguments", - "HfApi", - "HfFolder", - "ModelSearchArguments", - "change_discussion_status", - "comment_discussion", - "create_commit", - "create_discussion", - "create_pull_request", - "create_repo", - "dataset_info", - "delete_file", - "delete_repo", - "edit_discussion_comment", - "get_dataset_tags", - "get_discussion_details", - "get_full_repo_name", - "get_model_tags", - "get_repo_discussions", - "list_datasets", - "list_metrics", - "list_models", - "list_repo_files", - "login", - "logout", - "merge_pull_request", - "model_info", - "move_repo", - "rename_discussion", - "repo_type_and_id_from_hf_id", - "set_access_token", - "space_info", - "unset_access_token", - "update_repo_visibility", - "upload_file", - "upload_folder", - "whoami", - ], - "hub_mixin": ["ModelHubMixin", "PyTorchModelHubMixin"], - "inference_api": ["InferenceApi"], - "keras_mixin": [ - "KerasModelHubMixin", - "from_pretrained_keras", - "push_to_hub_keras", - "save_pretrained_keras", - ], - "repository": ["Repository"], - "_snapshot_download": ["snapshot_download"], - "utils": [ - "logging", - "CachedFileInfo", - "CachedRepoInfo", - "CachedRevisionInfo", - "CorruptedCacheException", - "DeleteCacheStrategy", - "HFCacheInfo", - "scan_cache_dir", - ], - "utils.endpoint_helpers": ["DatasetFilter", "ModelFilter"], - "repocard": [ - "metadata_eval_result", - "metadata_load", - "metadata_save", - "metadata_update", - "ModelCard", - "DatasetCard", - ], - "community": [ - "Discussion", - "DiscussionWithDetails", - "DiscussionEvent", - "DiscussionComment", - "DiscussionStatusChange", - "DiscussionCommit", - "DiscussionTitleChange", - ], - "repocard_data": ["CardData", "ModelCardData", "DatasetCardData", "EvalResult"], - }, + __name__, submodules=[], submod_attrs=_SUBMOD_ATTRS ) + +# WARNING: any content below this statement is generated automatically. Any manual edit +# will be lost when re-generating this file ! +# +# To update the static imports, please run the following command and commit the changes. +# ``` +# # Use script +# python utils/check_static_imports.py --update-file +# +# # Or run style on codebase +# make style +# ``` +if TYPE_CHECKING: # pragma: no cover + from ._snapshot_download import snapshot_download # noqa: F401 + from .commands.user import notebook_login # noqa: F401 + from .community import Discussion # noqa: F401 + from .community import DiscussionComment # noqa: F401 + from .community import DiscussionCommit # noqa: F401 + from .community import DiscussionEvent # noqa: F401 + from .community import DiscussionStatusChange # noqa: F401 + from .community import DiscussionTitleChange # noqa: F401 + from .community import DiscussionWithDetails # noqa: F401 + from .constants import CONFIG_NAME # noqa: F401 + from .constants import FLAX_WEIGHTS_NAME # noqa: F401 + from .constants import HUGGINGFACE_CO_URL_HOME # noqa: F401 + from .constants import HUGGINGFACE_CO_URL_TEMPLATE # noqa: F401 + from .constants import PYTORCH_WEIGHTS_NAME # noqa: F401 + from .constants import REPO_TYPE_DATASET # noqa: F401 + from .constants import REPO_TYPE_MODEL # noqa: F401 + from .constants import REPO_TYPE_SPACE # noqa: F401 + from .constants import TF2_WEIGHTS_NAME # noqa: F401 + from .constants import TF_WEIGHTS_NAME # noqa: F401 + from .fastai_utils import _save_pretrained_fastai # noqa: F401 + from .fastai_utils import from_pretrained_fastai # noqa: F401 + from .fastai_utils import push_to_hub_fastai # noqa: F401 + from .file_download import cached_download # noqa: F401 + from .file_download import hf_hub_download # noqa: F401 + from .file_download import hf_hub_url # noqa: F401 + from .file_download import try_to_load_from_cache # noqa: F401 + from .hf_api import CommitOperation # noqa: F401 + from .hf_api import CommitOperationAdd # noqa: F401 + from .hf_api import CommitOperationDelete # noqa: F401 + from .hf_api import DatasetSearchArguments # noqa: F401 + from .hf_api import HfApi # noqa: F401 + from .hf_api import HfFolder # noqa: F401 + from .hf_api import ModelSearchArguments # noqa: F401 + from .hf_api import change_discussion_status # noqa: F401 + from .hf_api import comment_discussion # noqa: F401 + from .hf_api import create_commit # noqa: F401 + from .hf_api import create_discussion # noqa: F401 + from .hf_api import create_pull_request # noqa: F401 + from .hf_api import create_repo # noqa: F401 + from .hf_api import dataset_info # noqa: F401 + from .hf_api import delete_file # noqa: F401 + from .hf_api import delete_repo # noqa: F401 + from .hf_api import edit_discussion_comment # noqa: F401 + from .hf_api import get_dataset_tags # noqa: F401 + from .hf_api import get_discussion_details # noqa: F401 + from .hf_api import get_full_repo_name # noqa: F401 + from .hf_api import get_model_tags # noqa: F401 + from .hf_api import get_repo_discussions # noqa: F401 + from .hf_api import list_datasets # noqa: F401 + from .hf_api import list_metrics # noqa: F401 + from .hf_api import list_models # noqa: F401 + from .hf_api import list_repo_files # noqa: F401 + from .hf_api import merge_pull_request # noqa: F401 + from .hf_api import model_info # noqa: F401 + from .hf_api import move_repo # noqa: F401 + from .hf_api import rename_discussion # noqa: F401 + from .hf_api import repo_type_and_id_from_hf_id # noqa: F401 + from .hf_api import set_access_token # noqa: F401 + from .hf_api import space_info # noqa: F401 + from .hf_api import unset_access_token # noqa: F401 + from .hf_api import update_repo_visibility # noqa: F401 + from .hf_api import upload_file # noqa: F401 + from .hf_api import upload_folder # noqa: F401 + from .hf_api import whoami # noqa: F401 + from .hub_mixin import ModelHubMixin # noqa: F401 + from .hub_mixin import PyTorchModelHubMixin # noqa: F401 + from .inference_api import InferenceApi # noqa: F401 + from .keras_mixin import KerasModelHubMixin # noqa: F401 + from .keras_mixin import from_pretrained_keras # noqa: F401 + from .keras_mixin import push_to_hub_keras # noqa: F401 + from .keras_mixin import save_pretrained_keras # noqa: F401 + from .repocard import DatasetCard # noqa: F401 + from .repocard import ModelCard # noqa: F401 + from .repocard import metadata_eval_result # noqa: F401 + from .repocard import metadata_load # noqa: F401 + from .repocard import metadata_save # noqa: F401 + from .repocard import metadata_update # noqa: F401 + from .repocard_data import CardData # noqa: F401 + from .repocard_data import DatasetCardData # noqa: F401 + from .repocard_data import EvalResult # noqa: F401 + from .repocard_data import ModelCardData # noqa: F401 + from .repository import Repository # noqa: F401 + from .utils import CachedFileInfo # noqa: F401 + from .utils import CachedRepoInfo # noqa: F401 + from .utils import CachedRevisionInfo # noqa: F401 + from .utils import CorruptedCacheException # noqa: F401 + from .utils import DeleteCacheStrategy # noqa: F401 + from .utils import HFCacheInfo # noqa: F401 + from .utils import logging # noqa: F401 + from .utils import scan_cache_dir # noqa: F401 + from .utils.endpoint_helpers import DatasetFilter # noqa: F401 + from .utils.endpoint_helpers import ModelFilter # noqa: F401 diff --git a/tests/test_init_lazy_loading.py b/tests/test_init_lazy_loading.py new file mode 100644 index 0000000000..54b747e479 --- /dev/null +++ b/tests/test_init_lazy_loading.py @@ -0,0 +1,43 @@ +import unittest + +import jedi + + +class TestHuggingfaceHubInit(unittest.TestCase): + def test_autocomplete_on_root_imports(self) -> None: + """Test autocomplete with `huggingface_hub` works with Jedi. + + Not all autocomplete systems are based on Jedi but if this one works we can + assume others do as well. + """ + source = """from huggingface_hub import c""" + script = jedi.Script(source, path="example.py") + completions = script.complete(1, len(source)) + + for completion in completions: + if completion.name == "create_commit": + # Assert `create_commit` is suggestion from `huggingface_hub` lib + self.assertEquals(completion.module_name, "huggingface_hub") + + # Assert autocomplete knows where `create_commit` lives + # It would not be the case with a dynamic import. + goto_list = completion.goto() + self.assertEquals(len(goto_list), 1) + + # Assert docstring is find. This means autocomplete can also provide + # the help section. + signature_list = goto_list[0].get_signatures() + self.assertEquals(len(signature_list), 1) + self.assertTrue( + signature_list[0] + .docstring() + .startswith("create_commit(self, repo_id: str") + ) + break + else: + self.fail( + "Jedi autocomplete did not suggest `create_commit` to complete the" + f" line `{source}`. It is most probable that static imports are not" + " correct in `./src/huggingface_hub/__init__.py`. Please run `make" + " style` to fix this." + ) diff --git a/utils/check_static_imports.py b/utils/check_static_imports.py new file mode 100644 index 0000000000..5107ad47d5 --- /dev/null +++ b/utils/check_static_imports.py @@ -0,0 +1,123 @@ +# coding=utf-8 +# Copyright 2022-present, the HuggingFace Inc. team. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Contains a tool to reformat static imports in `huggingface_hub.__init__.py`.""" +import argparse +import re +from pathlib import Path +from typing import NoReturn + +import isort +from huggingface_hub import _SUBMOD_ATTRS + + +INIT_FILE_PATH = Path(__file__).parents[1] / "src" / "huggingface_hub" / "__init__.py" +SETUP_CFG_PATH = Path(__file__).parents[1] / "setup.cfg" + +IF_TYPE_CHECKING_LINE = "\nif TYPE_CHECKING: # pragma: no cover\n" +SUBMOD_ATTRS_PATTERN = re.compile("_SUBMOD_ATTRS = {[^}]+}") # match the all dict + + +def check_static_imports(update_file: bool) -> NoReturn: + """Check all imports are made twice (1 in lazy-loading and 1 in static checks). + + For more explanations, see `./src/huggingface_hub/__init__.py`. + This script is used in the `make style` and `make quality` checks. + """ + with INIT_FILE_PATH.open() as f: + init_content = f.read() + + # Get first half of the `__init__.py` file. + # WARNING: Content after this part will be entirely re-generated which means + # human-edited changes will be lost ! + init_content_before_static_checks = init_content.split(IF_TYPE_CHECKING_LINE)[0] + + # Search and replace `_SUBMOD_ATTRS` dictionary definition. This ensures modules + # and functions that can be lazy-loaded are alphabetically ordered for readability. + if SUBMOD_ATTRS_PATTERN.search(init_content_before_static_checks) is None: + print( + "Error: _SUBMOD_ATTRS dictionary definition not found in" + " `./src/huggingface_hub/__init__.py`." + ) + exit(1) + + _submod_attrs_definition = ( + "_SUBMOD_ATTRS = {\n" + + "\n".join( + f' "{module}": [\n' + + "\n".join(f' "{attr}",' for attr in sorted(_SUBMOD_ATTRS[module])) + + "\n ]," + for module in sorted(_SUBMOD_ATTRS.keys()) + ) + + "\n}" + ) + reordered_content_before_static_checks = SUBMOD_ATTRS_PATTERN.sub( + _submod_attrs_definition, init_content_before_static_checks + ) + + # Generate the static imports given the `_SUBMOD_ATTRS` dictionary. + static_imports = [ + f" from .{module} import {attr} # noqa: F401" + for module, attributes in _SUBMOD_ATTRS.items() + for attr in attributes + ] + + # Generate the expected `__init__.py` file content and apply formatter on it. + expected_init_content = isort.code( + reordered_content_before_static_checks + + IF_TYPE_CHECKING_LINE + + "\n".join(static_imports) + + "\n", + config=isort.Config(settings_path=SETUP_CFG_PATH), + ) + + # If expected `__init__.py` content is different, test fails. If '--update-init-file' + # is used, `__init__.py` file is updated before the test fails. + if init_content != expected_init_content: + if update_file: + with INIT_FILE_PATH.open("w") as f: + f.write(expected_init_content) + + print( + "✅ Imports have been updated in `./src/huggingface_hub/__init__.py`." + "\n Please make sure the changes are accurate and commit them." + ) + exit(0) + else: + print( + "❌ Expected content mismatch in" + " `./src/huggingface_hub/__init__.py`.\n It is most likely that you" + " added a module/function to `_SUBMOD_ATTRS` and did not update the" + " 'static import'-part.\n Please run `make style` or `python" + " utils/check_static_imports.py --update-file`." + ) + exit(1) + + print("✅ All good!") + exit(0) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument( + "--update-file", + action="store_true", + help=( + "Whether to fix `./src/huggingface_hub/__init__.py` if a change is" + " detected." + ), + ) + args = parser.parse_args() + + check_static_imports(update_file=args.update_file)