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

Better support for mypy. #6496

Open
trivialfis opened this issue Dec 13, 2020 · 20 comments
Open

Better support for mypy. #6496

trivialfis opened this issue Dec 13, 2020 · 20 comments

Comments

@trivialfis
Copy link
Member

trivialfis commented Dec 13, 2020

Currently many more Python projects like dask and optuna are using Python type hints. With the Python package of xgboost gaining more and more features, we should also adopt mypy as a safe guard against some type errors and for better code documentation.

@trivialfis
Copy link
Member Author

I marked this as good first issue. Adding type checks for Python is an incremental process, parameter by parameter and function by function. It would be a good chance to review existing code base and make improvement during the process. I have made some progress on #6519 with the dask module, which is by far the most dynamic module in xgboost. Other modules can use it as a base line.

Contributions are welcomed!

@jjwang01
Copy link

Hi, I was wondering if I could work on this as a first issue. Thanks!

@trivialfis
Copy link
Member Author

@jjwang01 Please do. ;-) ping me if you need any help

@jjwang01
Copy link

Is it alright if I split this into multiple PRs?

@trivialfis
Copy link
Member Author

@jjwang01 As many as you need. In fact, it's a good idea to have multiple PRs so we can communicate better around details.

@jjwang01
Copy link

@trivialfis Is there some sort of local build script that I can use to test locally rather than after submitting a commit to PR?

@jjwang01
Copy link

jjwang01 commented Feb 2, 2022

Hi, just wanted to follow up on this comment.

@trivialfis
Copy link
Member Author

Hi, sorry for missing the message.

  1. Install mypy in your python envorinment.
  2. cd python-package in xgboost source directory.
  3. Run mypy .

@trivialfis
Copy link
Member Author

The only module that's lacking the support for mypy is the pyspark interface now.

@trivialfis
Copy link
Member Author

trivialfis commented Jul 28, 2022

Notes:

@trivialfis
Copy link
Member Author

Will not cover pyspark at 2.0. mypy is not ready for pyspark's deep inheritance style.

@michael-gendy-mention-me
Copy link
Contributor

michael-gendy-mention-me commented May 3, 2023

Is there anything else to do here for now? Referring to the comments 'The only module that's lacking the support for mypy is the pyspark interface now' followed by 'Will not cover pyspark at 2.0'. If there's anymore locations where this is needed I'm happy to pick it up

@trivialfis
Copy link
Member Author

I think we will keep it opened until xgboost is fully covered, including the pyspark module. You are more than welcome to work on it. I think there's still some parts of the pyspark interface can be typed.

@SANTHOSH-MAMIDISETTI
Copy link

I am new to open source and am willing to work over it , provided someone helps me in knowing what has to be done ,
@trivialfis is this still going on ? or are there any other issues that I might be able to work on ,

@trivialfis
Copy link
Member Author

Hi, thank you for volunteering @SANTHOSH-MAMIDISETTI ! Currently, XGBoost has type hint for the core Python library, but not yet for most of the demos and tests. In addition, the PySpark module is not yet typed. Feel free to pick an untyped file and start adding type hints.

@SANTHOSH-MAMIDISETTI
Copy link

Hi, thank you for volunteering @SANTHOSH-MAMIDISETTI ! Currently, XGBoost has type hint for the core Python library, but not yet for most of the demos and tests. In addition, the PySpark module is not yet typed. Feel free to pick an untyped file and start adding type hints.

Dear @trivialfis ,

Thank you for your prompt response. I am excited to contribute to the XGBoost project and would like to request assignment for the issue regarding adding type hints to the PySpark module, as mentioned earlier.

As a beginner in open source, I anticipate that I may encounter challenges along the way. Therefore, I would appreciate some guidance on whom I can reach out to for assistance when faced with difficulties. It would be reassuring to know that there is a support system in place to help newcomers like myself navigate any obstacles that may arise during the contribution process.

Thank you once again for the opportunity to contribute. I am eager to make a meaningful impact on the project.

Sincerely,
@SANTHOSH-MAMIDISETTI

@trivialfis
Copy link
Member Author

trivialfis commented May 15, 2023

Hi @SANTHOSH-MAMIDISETTI , there's an on-going effort for the pyspark module here: #9156

I would appreciate some guidance on whom I can reach out to for assistance when faced with difficulties.

Feel free to ping me or @hcho3 for assistance when needed. I should be able to reply unless away from my working devices.
.

As a beginner in open source,

Hope that you enjoy the journey!

@michael-gendy-mention-me
Copy link
Contributor

michael-gendy-mention-me commented May 22, 2023

Looks like I was too slow off the mark 😄 - is there anything left to type for the pyspark module? Or elsewhere for that matter? As far as I can tell it's all done

@trivialfis
Copy link
Member Author

trivialfis commented May 22, 2023

Tons of! Examples under the demo directory, tests under the tests directory. Also, the core package contains many use of type:ignore and the Any, removing them can help make the code more rigorous.

@michael-gendy-mention-me
Copy link
Contributor

michael-gendy-mention-me commented May 24, 2023

Thanks a lot - will contribute on this PR to try and remove the type: ignore hint. This will be my first attempt at an open source contribution so would value some guidance on if I'm on the right track

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

4 participants