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

[python-package] type hints in python package #3756

Open
10 of 12 tasks
jameslamb opened this issue Jan 13, 2021 · 13 comments
Open
10 of 12 tasks

[python-package] type hints in python package #3756

jameslamb opened this issue Jan 13, 2021 · 13 comments

Comments

@jameslamb
Copy link
Collaborator

jameslamb commented Jan 13, 2021

Request for Comment

#3581 officially ended support for Python 2 in the Python package.

Now that Python 2 support has been ended, I'd like to propose adding type hints to the library (https://docs.python.org/3/library/typing.html).

Benefits of adding type hints:

  • serves as additional documentation about the types of arguments and return types of functions
  • allows us to enable mypy, a static type checker for Python that will catch issues like "this function call can return None but you're accessing a property of the result as if it isn't None"
  • allows other libraries that integrate with LightGBM (like optuna) to catch issues in their own code using mypy

I'm opening this Request for Comment to get thoughts on this narrow proposal:

would you support a PR where I add type hints to the code in the https://github.com/microsoft/LightGBM/blob/master/python-package/lightgbm/dask.py?

Status: Accepted

If maintainers agree to this proposal, we could add a "good first issue" about adding hints for other modules and could talk about mypy separately, at some point in the future.

This was originally opened as a request for comment, but it's been accepted and we're now accepting contributions to add type hints in the Python package!

How to contribute

Anyone is welcome to submit a pull request adding type hints to places in the Python package that are missing them!

Please limit pull requests to 1 function/method per pull request, to make them easier to review.

Handling line breaks

If adding hints to a method or function greatly increases the length of its lines, break each argument onto a separate line and have the closing parenthesis begin its own line, vertically aligned with the keyword def. Like this:

# before
def make_ranking(n_samples=100, n_features=20, n_informative=5, gmax=2,
                 group=None, random_gs=False, avg_gs=10, random_state=0):

# after
def make_ranking(
    n_samples: int = 100,
    n_features: int = 20,
    n_informative: int = 5,
    gmax: int = 2,
    group: Optional[List[int]] = None,
    random_gs: bool = False,
    avg_gs: int = 10,
    random_state: int = 0
) -> Tuple[pd.DataFrame, pd.DataFrame]:

references that may help you

How this improves LightGBM

Type hints in the code serve as additional documentation of the expected input and output types for functions and methods.

Having richer type hints can help static analyzers like mypy catch bugs that otherwise might not be caught by LightGBM's unit tests.

@StrikerRUS
Copy link
Collaborator

I'm +1.
This shouldn't be hard to add type hints, because for all public API functions (and even some internal ones) all types are already specified in docstrings.

@trivialfis
Copy link

xgboost has been adding type hints gradually and the dask module is completely typed. From my experience it's quite useful in both documentation and type checks.

@jameslamb
Copy link
Collaborator Author

jameslamb commented Feb 16, 2021

I did the following this morning to make this more friendly for outside contributors:

  • replaced "[RFC]" in the title with "[python-package]"
  • removed label "question"
  • removed assignees
  • added more details in the description to help contributors looking to submit PRs that add type hints

Anyone reading this is welcome to contribute! Thanks for helping to improve LightGBM.

@kunjshukla
Copy link

I would like to take this issue up if it's not closed

@jameslamb
Copy link
Collaborator Author

@kunjshukla sure, pull requests are welcome. Please make them small and focused... don't try to submit one large PR that adds type hints in all the remaining places missing them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests