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

Use latest pylint + Remove outdated docstring #586

Merged
merged 3 commits into from
Sep 28, 2024
Merged
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
16 changes: 8 additions & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v4.6.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-added-large-files
args: ["--maxkb=4000"]
- repo: https://github.com/psf/black
rev: 24.2.0
rev: 24.8.0
hooks:
- id: black
- repo: https://github.com/pycqa/isort
Expand All @@ -19,12 +19,12 @@ repos:
- id: isort
args: ["--profile", "black", "--filter-files"]
- repo: https://github.com/MarcoGorelli/cython-lint
rev: v0.16.0
rev: v0.16.2
hooks:
- id: cython-lint
- id: double-quote-cython-strings
- repo: https://github.com/PyCQA/flake8
rev: 7.0.0
rev: 7.1.1
hooks:
- id: flake8
args: [--config=.flake8]
Expand All @@ -40,8 +40,8 @@ repos:
language: python
types: [python]
additional_dependencies: [
pylint, hatchling, pytest, scikit-learn, hypothesis, pandas, treelite,
lightgbm, xgboost, tqdm
pylint>=3.3.1, hatchling, pytest, scikit-learn>=1.5.2, hypothesis, pandas, treelite,
lightgbm, xgboost>=2.1.1, tqdm
]
- repo: https://github.com/pocc/pre-commit-hooks
rev: v1.3.5
Expand All @@ -60,12 +60,12 @@ repos:
additional_dependencies: [cpplint]
types_or: [c++]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.8.0
rev: v1.11.2
hooks:
- id: mypy
additional_dependencies: [types-setuptools, numpy]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.4
rev: v0.6.8
hooks:
- id: ruff
args: ["--config", "python/pyproject.toml"]
1 change: 0 additions & 1 deletion dev/change_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def update_pypkg(
rc_ver: Optional[int] = None,
) -> None:
"""Change version in the Python package"""
# pylint: disable=too-many-arguments
version = f"{major}.{minor}.{patch}"
if is_rc:
assert rc_ver
Expand Down
21 changes: 13 additions & 8 deletions dev/prepare_pypi_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
tqdm, sh are required to run this script.
"""

# pylint: disable=W0603,C0103,too-many-arguments
# pylint: disable=W0603,C0103

import argparse
import os
Expand Down Expand Up @@ -51,6 +51,7 @@ def latest_hash() -> str:


def download_wheels(
*,
platforms: List[str],
url_prefix: str,
dest_dir: str,
Expand Down Expand Up @@ -93,7 +94,11 @@ def download_py_packages(version_str: str, commit_hash: str) -> None:
src_filename_prefix = f"{pkg}-{version_str}%2B{commit_hash}-py3-none-"
target_filename_prefix = f"{pkg}-{version_str}-py3-none-"
filenames = download_wheels(
platforms, PREFIX, dest_dir, src_filename_prefix, target_filename_prefix
platforms=platforms,
url_prefix=PREFIX,
dest_dir=dest_dir,
src_filename_prefix=src_filename_prefix,
target_filename_prefix=target_filename_prefix,
)
print(f"List of downloaded wheels: {filenames}\n")

Expand All @@ -102,12 +107,12 @@ def download_py_packages(version_str: str, commit_hash: str) -> None:
src_filename_prefix = f"{pkg}-{version_str}%2B{commit_hash}"
target_filename_prefix = f"{pkg}-{version_str}"
filenames = download_wheels(
[""],
PREFIX,
dest_dir,
src_filename_prefix,
target_filename_prefix,
"tar.gz",
platforms=[""],
url_prefix=PREFIX,
dest_dir=dest_dir,
src_filename_prefix=src_filename_prefix,
target_filename_prefix=target_filename_prefix,
ext="tar.gz",
)
print(f"List of downloaded sdist: {filenames}\n")
print(
Expand Down
2 changes: 1 addition & 1 deletion python/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ extension-pkg-whitelist=numpy

load-plugins=pylint.extensions.no_self_use

disable=unexpected-special-method-signature,too-many-nested-blocks,useless-object-inheritance,import-outside-toplevel,unsubscriptable-object,attribute-defined-outside-init,unbalanced-tuple-unpacking,too-many-lines,duplicate-code
disable=unexpected-special-method-signature,too-many-nested-blocks,useless-object-inheritance,import-outside-toplevel,unsubscriptable-object,attribute-defined-outside-init,unbalanced-tuple-unpacking,too-many-lines,duplicate-code,too-many-arguments

dummy-variables-rgx=(unused|)_.*

Expand Down
12 changes: 8 additions & 4 deletions python/treelite/model_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from .core import _LIB, _check_call
from .model import Model
from .util import c_array, c_str
from .util import c_array, c_str, deprecate_positional_args


@dataclasses.dataclass
Expand Down Expand Up @@ -151,7 +151,7 @@ def __init__(
postprocessor: PostProcessorFunc,
base_scores: List[float],
attributes: Optional[Dict[Any, Any]] = None,
): # pylint: disable=R0913
):
self._handle = None

handle = ctypes.c_void_p()
Expand Down Expand Up @@ -211,15 +211,17 @@ def end_node(self):
"""End the current node"""
_check_call(_LIB.TreeliteModelBuilderEndNode(self.handle))

@deprecate_positional_args
def numerical_test(
self,
feature_id: int,
threshold: float,
*,
default_left: bool,
opname: str,
left_child_key: int,
right_child_key: int,
): # pylint: disable=R0913
):
"""
Declare the current node as a numerical test node, where the test is of form
[feature value] [op] [threshold]. Data points for which the test evaluates to True
Expand Down Expand Up @@ -253,15 +255,17 @@ def numerical_test(
)
)

@deprecate_positional_args
def categorical_test(
self,
feature_id: int,
*,
default_left: bool,
category_list: List[int],
category_list_right_child: bool,
left_child_key: int,
right_child_key: int,
): # pylint: disable=R0913
):
"""
Declare the current node as a categorical test node, where the test is of form
[feature value] \\in [category list].
Expand Down
12 changes: 8 additions & 4 deletions python/treelite/model_builder_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .model_builder import Metadata
from .model_builder import ModelBuilder as ModelBuilderNew
from .model_builder import PostProcessorFunc, TreeAnnotation
from .util import deprecate_positional_args


@dataclasses.dataclass
Expand Down Expand Up @@ -138,12 +139,13 @@ def set_leaf_node(
)
self.node = _LeafNode(leaf_value=leaf_value)

# pylint: disable=R0913
@deprecate_positional_args
def set_numerical_test_node(
self,
feature_id: int,
opname: str,
threshold: float,
*,
default_left: bool,
left_child_key: int,
right_child_key: int,
Expand Down Expand Up @@ -204,11 +206,12 @@ def set_numerical_test_node(
)
self.empty = False

# pylint: disable=R0913
@deprecate_positional_args
def set_categorical_test_node(
self,
feature_id: int,
left_categories: List[int],
*,
default_left: bool,
left_child_key: int,
right_child_key: int,
Expand Down Expand Up @@ -332,19 +335,20 @@ def _set_root(self, node_key: Optional[int]):
assert node_key is not None
self.root_key = node_key

@deprecate_positional_args
def __init__(
self,
*,
num_feature: int,
num_class: int = 1,
average_tree_output: bool = False,
threshold_type: str = "float32",
leaf_output_type: str = "float32",
*,
pred_transform: str = "identity",
sigmoid_alpha: float = 1.0,
ratio_c: float = 1.0,
global_bias: float = 0.0,
): # pylint: disable=R0913
):
warnings.warn(
"treelite.ModelBuilder is deprecated and will be removed in Treelite 4.1. "
"Please use treelite.model_builder.ModelBuilder class instead. The new model builder "
Expand Down
8 changes: 0 additions & 8 deletions python/treelite/sklearn/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,6 @@ def import_model(sklearn_model):

import treelite.sklearn
model = treelite.sklearn.import_model(clf)

Note
----
This function does not yet support categorical splits in
:py:class:`~sklearn.ensemble.HistGradientBoostingRegressor` and
:py:class:`~sklearn.ensemble.HistGradientBoostingClassifier`.
If you are using either estimator types, make sure that all
test nodes have numerical test conditions.
"""
try:
from sklearn.dummy import DummyClassifier, DummyRegressor
Expand Down
80 changes: 80 additions & 0 deletions python/treelite/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
"""

import ctypes
import warnings
from functools import wraps
from inspect import Parameter, signature
from typing import Any, Callable, TypeVar

import numpy as np

Expand Down Expand Up @@ -47,3 +51,79 @@
ndarray.ctypes.data_as(*) to expose underlying buffer as C pointer.
"""
return (ctype * len(values))(*values)


_T = TypeVar("_T")


# Notice for `require_keyword_args`
# Authors: Olivier Grisel
# Gael Varoquaux
# Andreas Mueller
# Lars Buitinck
# Alexandre Gramfort
# Nicolas Tresegnie
# Sylvain Marie
# License: BSD 3 clause
def _require_keyword_args(
error: bool,
) -> Callable[[Callable[..., _T]], Callable[..., _T]]:
"""Decorator for methods that issues warnings for positional arguments

Using the keyword-only argument syntax in pep 3102, arguments after the
* will issue a warning or error when passed as a positional argument.

Modified from sklearn utils.validation.

Parameters
----------
error :
Whether to throw an error or raise a warning.
"""

def throw_if(func: Callable[..., _T]) -> Callable[..., _T]:
"""Throw an error/warning if there are positional arguments after the asterisk.

Parameters
----------
func :
function to check arguments on.

"""
sig = signature(func)
kwonly_args = []
all_args = []

for name, param in sig.parameters.items():
if param.kind == Parameter.POSITIONAL_OR_KEYWORD:
all_args.append(name)
elif param.kind == Parameter.KEYWORD_ONLY:
kwonly_args.append(name)

@wraps(func)
def inner_f(*args: Any, **kwargs: Any) -> _T:
extra_args = len(args) - len(all_args)
if not all_args and extra_args > 0: # keyword argument only
raise TypeError("Keyword argument is required.")

Check warning on line 107 in python/treelite/util.py

View check run for this annotation

Codecov / codecov/patch

python/treelite/util.py#L107

Added line #L107 was not covered by tests

if extra_args > 0:
# ignore first 'self' argument for instance methods
args_msg = [

Check warning on line 111 in python/treelite/util.py

View check run for this annotation

Codecov / codecov/patch

python/treelite/util.py#L111

Added line #L111 was not covered by tests
f"{name}"
for name, _ in zip(kwonly_args[:extra_args], args[-extra_args:])
]
# pylint: disable=consider-using-f-string
msg = "Pass `{}` as keyword args.".format(", ".join(args_msg))
if error:
raise TypeError(msg)
warnings.warn(msg, FutureWarning)

Check warning on line 119 in python/treelite/util.py

View check run for this annotation

Codecov / codecov/patch

python/treelite/util.py#L116-L119

Added lines #L116 - L119 were not covered by tests
for k, arg in zip(sig.parameters, args):
kwargs[k] = arg
return func(**kwargs)

return inner_f

return throw_if


deprecate_positional_args = _require_keyword_args(False)
3 changes: 1 addition & 2 deletions tests/python/hypothesis_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def standard_classification_datasets(
shuffle=just(True),
random_state=None,
):
# pylint: disable=too-many-locals, too-many-arguments
# pylint: disable=too-many-locals
"""
Returns a strategy to generate classification problem input datasets.
Note:
Expand Down Expand Up @@ -246,7 +246,6 @@ def standard_regression_datasets(
A tuple of search strategies for arrays subject to the constraints of
the provided parameters.
"""
# pylint: disable=too-many-arguments
n_features_ = draw(n_features)
if n_informative is None:
n_informative = just(max(min(n_features_, 1), int(0.1 * n_features_)))
Expand Down
3 changes: 2 additions & 1 deletion tests/python/test_lightgbm_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,15 @@ def test_lightgbm_binary_classification(
)
@settings(**standard_settings())
def test_lightgbm_multiclass_classification(
*,
dataset,
objective,
boosting_type,
num_boost_round,
use_categorical,
callback,
):
# pylint: disable=too-many-locals,too-many-arguments
# pylint: disable=too-many-locals
"""Test LightGBM multi-class classifier"""
X, y = dataset
num_class = np.max(y) + 1
Expand Down
Loading
Loading