Skip to content

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.

👍

}
}

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