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 in .\python-package\lightgbm\sklearn.py #4181

Closed
wants to merge 2 commits into from
Closed

Migrate to f-strings in .\python-package\lightgbm\sklearn.py #4181

wants to merge 2 commits into from

Conversation

akshitadixit
Copy link
Contributor

Related to #4136

I did not touch the normal strings since the issue quoted:

Migrate all Python code from old-fashioned format() functions, formatting % operators and simple concatenations (+) to modern f-strings

Please let me know of possible edits.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for taking this on! Before we review the changes to sklearn.py, please see my note about removing the unrelated changes from #4175.

@@ -32,27 +32,34 @@ Some explanatory pictures:
.. image:: ./_static/images/gcc-table.png
:align: center
:target: ./_static/images/gcc-table.png
:alt: picture comparing depth with number of threads with each implementation where red color means bad combination and greener areas mean good combination
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these unrelated changes may have shown up because you created #4175 from master instead of a separate branch on your fork.

Could you please remove them?

I think you'll need to do the following.

  1. close Add alt text for gcc-tips #4175
  2. create a new branch from your local master
git checkout master
git checkout -b docs/gcc-alt-txt
git push origin docs/gcc-alt-txt
  1. Open a new pull request for the gcc-tips documentation, from the branch docs/gcc-alt-txt
  2. reset master locally and on your fork to LightGBM's master
# delete local master branch
git branch -D master

# add LightGBM official repo as another remo
git remote add upstream git@github.com:microsoft/LightGBM.git

# get master branch from LightGBM official repo
git fetch upstream master
git checkout master

# overwrite your fork's "master" branch with the official LightGBM one
git push origin master --force
  1. Rebase this branch to that new master
git checkout akshita_fstrings
git rebase master
git push origin akshita_fstrings --force

Let me know if you have any questions about that, we're here to help!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing that right away!

@akshitadixit
Copy link
Contributor Author

Sorry about the mess, I will create fresh PRs

@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 23, 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.

2 participants