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

I#163 dataset drop #209

Merged
merged 3 commits into from
Nov 21, 2017

Conversation

frosforever
Copy link
Contributor

@frosforever frosforever commented Nov 17, 2017

Connects to #163

Suffers from the same problem as #207

@codecov-io
Copy link

codecov-io commented Nov 17, 2017

Codecov Report

Merging #209 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
+ Coverage   95.71%   95.72%   +0.01%     
==========================================
  Files          39       39              
  Lines         793      796       +3     
  Branches       11       13       +2     
==========================================
+ Hits          759      762       +3     
  Misses         34       34
Impacted Files Coverage Δ
...ataset/src/main/scala/frameless/TypedDataset.scala 93.38% <100%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daca214...2dcd565. Read the comment docs.

@OlivierBlanvillain OlivierBlanvillain changed the title I#164 dataset drop I#163 dataset drop Nov 17, 2017
Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain left a comment

Choose a reason for hiding this comment

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

👍

@@ -605,6 +606,29 @@ class TypedDataset[T] protected[frameless](val dataset: Dataset[T])(implicit val
}
}

def drop[
Copy link
Contributor

Choose a reason for hiding this comment

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

/** Returns a new Dataset with a column dropped.
  *
  * {{{ maybe give an example? }}}
  *
  * apache/spark
  */

import org.scalacheck.Prop._

class DropTest extends TypedDatasetSuite {
test("drop five columns") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could add a test dropping the only the first column, and another one dropping only the last column

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can make it more explicit by making separate tests but those two cases are already covered in this test by dropping 'a which is the first column and then _4 etc which are the last ones

@OlivierBlanvillain OlivierBlanvillain mentioned this pull request Nov 17, 2017
76 tasks
@imarios
Copy link
Contributor

imarios commented Nov 17, 2017

LGTM as well! Nice work @frosforever. Let us know when you add everything you want to add for the PR. Thank you!

@frosforever
Copy link
Contributor Author

Thanks @imarios! I think this is done now.

@frosforever
Copy link
Contributor Author

hmm, unclear to me why travis failed. it tests correctly locally.

@imarios
Copy link
Contributor

imarios commented Nov 18, 2017

might be transient thing. restarting travis.

@imarios imarios closed this Nov 18, 2017
@imarios imarios reopened this Nov 18, 2017
@OlivierBlanvillain
Copy link
Contributor

Thanks for the PR!

@OlivierBlanvillain OlivierBlanvillain merged commit 2709c1d into typelevel:master Nov 21, 2017
@frosforever frosforever deleted the i#164-dataset-drop branch November 26, 2017 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants