Skip to content
This repository was archived by the owner on Dec 10, 2018. It is now read-only.

Make flatten stricter in data preparation#111

Merged
kortschak merged 1 commit intomasterfrom
capped
Mar 16, 2015
Merged

Make flatten stricter in data preparation#111
kortschak merged 1 commit intomasterfrom
capped

Conversation

@kortschak
Copy link
Copy Markdown
Member

  • Panic on ragged input or rowless data.
  • Build data slice that has exact cap - this is what NewDense does, so we should match that.

This change catches a bug in (*Dense).ColView that will be fixed with a PR to be merged later today (#110).

@kortschak kortschak force-pushed the capped branch 2 times, most recently from 435da5f to 8d526f9 Compare March 7, 2015 01:17
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe these should just be "flatten: no row".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SGTM

@btracey
Copy link
Copy Markdown
Member

btracey commented Mar 8, 2015

LGTM besides minor nit.

* Panic on ragged input or rowless data.
* Build data slice that has exact cap - this is what NewDense does, so
  we should match that.

Also add code to TestRowColView that should have been added as
TestRowColViewFullCap in #110.
@kortschak
Copy link
Copy Markdown
Member Author

@btracey PTAL

@btracey
Copy link
Copy Markdown
Member

btracey commented Mar 16, 2015

LGTM

@kortschak kortschak merged commit 51ad6ff into master Mar 16, 2015
@kortschak kortschak deleted the capped branch March 16, 2015 01:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants