Post: mlr3 and OpenML - Moneyball use case

mlr-org / mlr3gallery

Case studies using mlr3

https://mlr3gallery.mlr-org.com

21 stars 9 forks source link

Post: mlr3 and OpenML - Moneyball use case #16

Closed pkopper closed 4 years ago

pkopper commented 4 years ago

Couldn't render other (the previous ones) use cases locally still searching for the error. @pat-s any idea? @danielsaggau please language & flow review. @pat-s, we should maybe discuss how to retrieve the data in the best possible manner from OpenML. Instead of the package usage we could also use this https://github.com/mlr-org/mlr3/issues/119 happy to receive feedback!

pkopper commented 4 years ago

Checks fail due to different use case

berndbischl commented 4 years ago

Checks fail due to different use case

I don't know what that sentence is supposed to mean

berndbischl commented 4 years ago

also: i am pretty sure you are not supposed to push about 50 files? @pat-s

pat-s commented 4 years ago

also: i am pretty sure you are not supposed to push about 50 files? @pat-s

Probably not.

Checks fail due to different use case

Me neither. I guess he refers to the fact that the CI run of this PR failed due to a different posts. This post is fine in master, so it's just an issue that master is not fully merged into this branch.

Such things (merging master) are really basic and it would be great if a bit more effort could go into this before just posting such a comment.

pkopper commented 4 years ago

@pat-s the branch is ahead 20 commits and behind 0... I left the comment to reassure with you only.

berndbischl commented 4 years ago

@pkopper seh ich das richtig dass das ding hier aus deiner sicht gut und fetig ist? daniel ist jetzt ja auch schon iteriert? weil dann les ich es jetzt auchmal, notfalls, pack ich das dann in einen neuen PR

pat-s commented 4 years ago

I've taken over and cleaned up.

Some comments:

I don't think every number in the text should be formatted as LaTeX math
You may want to use the mlr-style next time (the post still does not fully do it, still <- in there)
Code output is way too much in many chunks
The description should have content that introduces/summarises the post, not a generic two-sentence introduction what mlr3
Why did you touch the imbalanced post in this PR?

berndbischl commented 4 years ago

ok, i read the post over the weekend. as Patrick commented on technical stuff, let me comment on content.

1) I like the data set / usecase in general. but: 2) I don't like that we conflate the interesting use-case with openml. that doesn't make sense. actually, to explain openml I would use a "boring" usecase. and to explain Moneyball as a task, I would cut out openml 3) I think the most-important aspect is missing! The usecase is basically explained for "how to do ML Blackbox modelling" and that doesn't add much new stuff. Assuming that this is an interesting data set, isn't the most exciting aspect to do some IML on this?

pat-s commented 4 years ago

Merging this now because it also contains some fixes for the master branch which I had to include to get this one working.

You can still do a review Bernd (and I thinks it's very much needed) and then open new PR for this.