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] add type hints on plotting code #5708

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Feb 13, 2023

Contributes to #3756.

Adds type hints on most other places in python-package/plotting.py that were missing them.

Also proposes some other changes that I feel make it easier to follow the flow of data through that module (which I think is closely related to the goal of adding type hints).

  • removes defaults in internal function plotting._to_graphviz()
    • to prevent "forgot to pass an argument through from the public interface" types of bugs
  • switches to keyword arguments for any function calls passing more than two arguments
    • to prevent "passed these arguments in the wrong order" types of bugs

Notes for Reviewers

I might propose a change similar to #5704 (moving add() up to the module level so it isn't redefined every time _to_graphviz() is called) in a future PR, but decided not to do that here so that it was clear, from inspecting at the diff, what I'm proposing changing with type hints and keywords arguments.

@jameslamb jameslamb marked this pull request as ready for review February 13, 2023 03:06
@jameslamb jameslamb changed the title WIP: [python-package] add type hints on plotting code [python-package] add type hints on plotting code Feb 13, 2023
@jameslamb jameslamb merged commit 9713ff4 into master Feb 14, 2023
@jameslamb jameslamb deleted the plotting-type-hints branch February 14, 2023 03:15
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed.
To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues
including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants