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

Migrate to f-strings for the Python code #4136

Closed
37 tasks done
StrikerRUS opened this issue Mar 29, 2021 · 35 comments
Closed
37 tasks done

Migrate to f-strings for the Python code #4136

StrikerRUS opened this issue Mar 29, 2021 · 35 comments

Comments

@StrikerRUS
Copy link
Collaborator

StrikerRUS commented Mar 29, 2021

Migrate all Python code from old-fashioned format() functions, formatting % operators and simple concatenations (+) to modern f-strings (brief guide). They are known to be the fastest approach and also increase code readability.

image
Source: https://cito.github.io/blog/f-strings/.

They are supported starting from Python 3.6 which is the oldest Python version LightGBM officially supports:

'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',

If you're interested in contributing, you do not need to fix all files at once. Pull requests that address all replacements in one file are mostly welcomed!

Refer to #4137 for the example PR.

@nishitjain1204
Copy link
Contributor

I can work on this if no one is assigned

@NovusEdge
Copy link
Contributor

I would like to take this one up if no one's assigned.

@StrikerRUS
Copy link
Collaborator Author

@coldkillerr @NovusEdge Thanks! Sure, please feel free to pick up any file(s) you like!

nishitjain1204 added a commit to nishitjain1204/LightGBM that referenced this issue Apr 4, 2021
nishitjain1204 added a commit to nishitjain1204/LightGBM that referenced this issue Apr 4, 2021
nishitjain1204 added a commit to nishitjain1204/LightGBM that referenced this issue Apr 4, 2021
jameslamb pushed a commit that referenced this issue Apr 12, 2021
* Migrated to f-strings for .\.nuget\create_nuget.py #4136

* Migrate to f-strings for the Python code for  docs/conf.py #4136

* add missing whitespace at ./docs/conf.py:79:14

* Revert "add missing whitespace at ./docs/conf.py:79:14"

This reverts commit 6020084.

* Revert "Migrate to f-strings for the Python code for  docs/conf.py #4136"

This reverts commit 6f28bd1.

* added the requested changes in .nuget/create_nuget.py

* added the requested changes

* Update .nuget/create_nuget.py

Co-authored-by: Nikita Titov <[email protected]>

* Update .nuget/create_nuget.py

Co-authored-by: Nikita Titov <[email protected]>

* Update .nuget/create_nuget.py

Co-authored-by: Nikita Titov <[email protected]>

Co-authored-by: Nikita Titov <[email protected]>
@akshitadixit
Copy link
Contributor

Hi may I please work on this?

@jameslamb
Copy link
Collaborator

@akshitadixit yes please! We'd be grateful for the help.

Please only submit pull requests for a single file, and please check the list of open pull requests at https://github.com/microsoft/LightGBM/pulls since there are a few related to this issue already open.

@jameslamb
Copy link
Collaborator

Oh also, the code in python-package is higher priority than the tests or examples.

@akshitadixit
Copy link
Contributor

Sure, I will keep all of this in mind.

@sayantan1410
Copy link
Contributor

Hi, may i work on this issue ?

@jameslamb
Copy link
Collaborator

Sure, thank you! Please only submit a pull request for one file at a time. @ me if you have any questions.

@sayantan1410
Copy link
Contributor

Thank you so much @jameslamb

@akshitadixit
Copy link
Contributor

akshitadixit commented May 12, 2021

Hey, please add a checkmark on the following files since they have no strings that need to be migrated:

  • .\python-package\lightgbm\compat.py
  • .\tests\python_package_test_init_.py
  • .\tests\python_package_test\utils.py

@jameslamb
Copy link
Collaborator

ah yes you're right, thanks very much @akshitadixit !

@sagnik1511
Copy link
Contributor

Hello, can I contribute to this issue ??
I want to update .\examples\python-guide\logistic_regression.py this file.

@jameslamb
Copy link
Collaborator

Yes please! Thanks for the help @sagnik1511

@sagnik1511
Copy link
Contributor

Thanks, @jameslamb for letting me collaborate !
Do check the PR.
Thank You :)

@sayantan1410
Copy link
Contributor

sayantan1410 commented Jun 15, 2021

Hey, May I work on this file: .\examples\python-guide\advanced_example.py ?
@jameslamb

@jameslamb
Copy link
Collaborator

@sayantan1410 thanks for all your help! You can work on any file mentioned in this issue.

@sayantan1410
Copy link
Contributor

Thank you !!
@jameslamb

@amanjha8100
Copy link
Contributor

amanjha8100 commented Jun 30, 2021

Hey, can I contribute to this issue?
.\examples\python-guide\plot_example.py
@jameslamb

@jameslamb
Copy link
Collaborator

Yes please, we'd be grateful for the help.

@amanjha8100
Copy link
Contributor

Thanks, @jameslamb for letting me contribute !
Do check the PR.
Thank You :)

@amanjha8100
Copy link
Contributor

amanjha8100 commented Jul 3, 2021

Hey @jameslamb is there any other file that needs changing, I am unable to find any? Do we need to use regex with f-strings in .\helpers\check_dynamic_dependencies.py

@jameslamb
Copy link
Collaborator

Hey @jameslamb is there any other file that needs changing, I am unable to find any

Thanks for looking! I agree that helpers/check_dynamic_dependencies.py doesn't have any strings that need to be changed.

I updated the list in this issue's description:

  • libpath.py - updated in [ci] use f-strings in libpath.py #4137
  • helpers/check_dynamic_dependencies.py - I don't see any string concatenation that needs to be updated
  • tests/python_package_test/test_sklearn.py - I don't see any string concatenation that needs to be updated

test_engine.py does have some places that need to be updated but those cases are somewhat complicated so I am going to take that one to close this issue.

Thanks so much for your help! We hope you'll contribute again in the future.

@amanjha8100
Copy link
Contributor

Great, it was fun contributing!

@github-actions
Copy link

This issue 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 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants