search

JuliaML / MLPreprocessing.jl

Other
15 stars 2 forks source link

Syntax fixes for Julia v0.6 #4

Closed abieler closed 6 years ago

Evizero commented 6 years ago

LGTM. only comment is that we should also increase the version requirement in REQUIRE

abieler commented 6 years ago

only bump up julia version or also StatsBase LearnBase DataFrames?

Evizero commented 6 years ago

I think the Julia version should be fine

abieler commented 6 years ago

Side note: Do you know if there is somewhere a boxcox transformation and such already implemented in julia? I did not find anything on a quick google search.

Evizero commented 6 years ago

Another minor comment is that travis will fail until the CI settings are updated. same with Appveyor. If you feel up to it you could push the versions there as well. travis should be simple but the appveyor structure changed since last we updated this and needs a bit of rewrite (see here for a new reference https://github.com/Evizero/Augmentor.jl/blob/master/appveyor.yml)

Evizero commented 6 years ago

don't think i saw a boxcox implementation somewhere yet, no

Evizero commented 6 years ago

looking good. lets see what CI says

abieler commented 6 years ago

Regarding boxcox. That would probably require an Optim.jl dependency if we want auto determination of lambda. Do we want to go that route?

Evizero commented 6 years ago

good question. I think for prototyping its not a problem (this package isn't registered yet anyway). Once it exists its easier to make decisions about keeping it here or moving it into its own little drop in package

abieler commented 6 years ago

so this looks good to me. only nightly tests failed.