-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[ci] Introduce typos
pre-commit hook
#6564
base: master
Are you sure you want to change the base?
Changes from all commits
494f01c
13f9123
a29bc49
4f7ecba
586f591
c4f9d06
b00243f
994b6cc
757d88b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
default.extend-ignore-re = [ | ||
"/Ot", | ||
"mis-alignment", | ||
"mis-spelled", | ||
"posix-seh-rt", | ||
] | ||
|
||
[default.extend-words] | ||
MAPE = "MAPE" | ||
datas = "datas" | ||
interprete = "interprete" | ||
mape = "mape" | ||
splitted = "splitted" | ||
|
||
[default.extend-identifiers] | ||
ERRORs = "ERRORs" | ||
GAM = "GAM" | ||
ND24s = "ND24s" | ||
WARNINGs = "WARNINGs" | ||
fullset = "fullset" | ||
thess = "thess" |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,9 +8,9 @@ These builds of LightGBM all train on the CPU. For GPU-enabled builds, see [the | |||||||||||||
|
||||||||||||||
Follow the general installation instructions [on the Docker site](https://docs.docker.com/install/): | ||||||||||||||
|
||||||||||||||
* [macOS](https://docs.docker.com/docker-for-mac/install/) | ||||||||||||||
* [Ubuntu](https://docs.docker.com/install/linux/docker-ce/ubuntu/) | ||||||||||||||
* [Windows](https://docs.docker.com/docker-for-windows/install/) | ||||||||||||||
- [macOS](https://docs.docker.com/docker-for-mac/install/) | ||||||||||||||
- [Ubuntu](https://docs.docker.com/install/linux/docker-ce/ubuntu/) | ||||||||||||||
- [Windows](https://docs.docker.com/docker-for-windows/install/) | ||||||||||||||
Comment on lines
+11
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Was this particular change actually the result of the Unless this is necessary to satisfy any of the project's linters, or unless it improves the rendering somehow, could you please revert them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was my local editor, reverted! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem, thanks.
Did you forget to push a commit? This is still showing up in the diff. |
||||||||||||||
|
||||||||||||||
## Using CLI Version of LightGBM via Docker | ||||||||||||||
|
||||||||||||||
|
@@ -55,7 +55,7 @@ After this runs, a LightGBM model can be found at `LightGBM-CLI-model.txt`. | |||||||||||||
|
||||||||||||||
For more details on how to configure and use the LightGBM CLI, see https://lightgbm.readthedocs.io/en/latest/Quick-Start.html. | ||||||||||||||
|
||||||||||||||
## Running the Python-package Сontainer | ||||||||||||||
## Running the Python-package Container | ||||||||||||||
jameslamb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
Build an image with the LightGBM Python package installed. | ||||||||||||||
|
||||||||||||||
|
@@ -114,7 +114,7 @@ docker run \ | |||||||||||||
python | ||||||||||||||
``` | ||||||||||||||
|
||||||||||||||
## Running the R-package Сontainer | ||||||||||||||
## Running the R-package Container | ||||||||||||||
|
||||||||||||||
Build an image with the LightGBM R package installed. | ||||||||||||||
|
||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except leaving "interprete"!
See #6564 (comment) for my proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use this thread to discuss that proposal.
Please remember that breaking the public API of an R package is more difficult than doing it for a Python package, because of CRAN's restrictions.
CRAN will check the "strong reverse dependencies" (anything including
lightgbm
inLinkingTo
,Depends
, orImports
, I think) and check if a new submission breaks any of their builds or tests. This is why for the v4.0.0 release, I went around to all the packages listed at https://cran.r-project.org/web/packages/lightgbm/index.html as depending on{lightgbm}
and made contributions to make them compatible with both v3.x and v4.x of{lightgbm}
.HOWEVER.... it does not look like any package on CRAN is using
lgb.interprete()
: https://github.com/search?q=org%3Acran%20%22lgb.interprete%22&type=codeSo I'm ok with @StrikerRUS 's proposal, which is:
lgb.interpret()
which just callslgb.interprete()
lgb.interprete()
lgb.interprete()
@borchero if you are not comfortable making those R changes (like updating the
NAMESPACE
file or writing roxygen comments), let me know and I can do this in a follow-up PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, to be honest, I forgot about this!
Thank God! 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@borchero can you please return to this?
Just revert everything about
lgb.interprete
and we can talk about it in a separate issue. All of the other changes are non-controversial and I'd like to get this typo hook running inpre-commit
to help with other development.