Conversation
Codecov Report
@@ Coverage Diff @@
## master #26 +/- ##
============================================
+ Coverage 76.19% 77.69% +1.49%
- Complexity 268 357 +89
============================================
Files 28 39 +11
Lines 899 1318 +419
Branches 79 120 +41
============================================
+ Hits 685 1024 +339
- Misses 171 228 +57
- Partials 43 66 +23
Continue to review full report at Codecov.
|
|
Rename the folder of the new provider from |
|
@shenggwang I've finished the review, please see comments. |
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/TestSchemas.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/SchemaUtils.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/TestParameters.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/CSVUtils.java
Outdated
Show resolved
Hide resolved
...ghtgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMModelCreator.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMSWIG.java
Outdated
Show resolved
Hide resolved
...lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/SWIGTrainResources.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/CSVUtils.java
Outdated
Show resolved
Hide resolved
AlbertoEAF
left a comment
There was a problem hiding this comment.
It looks good @shenggwang.
In fact great work with the additions of "final" and fixes in the docstrings.
The visibility modifier changes should be reverted though.
We were extremely careful and purposefully expose only what's needed. For instance, many internal-use-only classes and properties are now public and user-visible, which should not happen.
I have one question for @JPDSousa , by dropping those TODO's, it means we'll have to maintain 2 versions of the provider? The open source and the internal?
openml-lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMSWIG.java
Outdated
Show resolved
Hide resolved
...main/java/com/feedzai/openml/provider/lightgbm/LightGBMBinaryClassificationModelTrainer.java
Outdated
Show resolved
Hide resolved
...tgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMDescriptorUtil.java
Show resolved
Hide resolved
openml-lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMSWIG.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMUtils.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/main/java/com/feedzai/openml/provider/lightgbm/SWIGResources.java
Outdated
Show resolved
Hide resolved
...rc/test/java/com/feedzai/openml/provider/lightgbm/LightGBMBinaryClassificationModelTest.java
Outdated
Show resolved
Hide resolved
.../java/com/feedzai/openml/provider/lightgbm/LightGBMBinaryClassificationModelTrainerTest.java
Outdated
Show resolved
Hide resolved
openml-lightgbm-provider/src/test/java/com/feedzai/openml/provider/lightgbm/TestSchemas.java
Outdated
Show resolved
Hide resolved
|
I have one final question, if we strip away the versions as above, then from now on how do we track the LightGBM provider version internally? |
I suppose that from now on, we are going to track internally the version of LightGBM on openml-java. |
openml-lightgbm/src/main/java/com/feedzai/openml/provider/lightgbm/LightGBMAlgorithms.java
Show resolved
Hide resolved
...lightgbm/src/test/java/com/feedzai/openml/provider/lightgbm/ClassifyUnknownCategoryTest.java
Outdated
Show resolved
Hide resolved
openml-h2o/src/test/java/com/feedzai/openml/h2o/algos/mocks/BindingRegularParameters.java
Outdated
Show resolved
Hide resolved
|
I've talked this over with @JPDSousa and we need to also move this repo |
I'd rather keep it apart if possible @ptraca , the make-lightgbm generates a lot of trash and temporary files and stuff that is not needed here. Also, both repos are agnostic to each other, and make-lightgbm serves to build artifacts for any version of LightGBM and Java LightGBM implementation, not only ours. |
|
@ptraca @JPDSousa @AlbertoEAF @paulojrp |
Port LightGBM provider from author Alberto Ferreira (alberto.ferreira@feedzai.com).