Enhance the DataFrame keyword constructor to recycle

[tshort]

This fixes an issue with the DataFrame constructor with keyword arguments. This comes up when using by, particularly with DataFramesMeta use. Here is the issue:

julia> DataFrame(a = 1:5, b = 1)
5x2 DataFrames.DataFrame
| Row | a | b |
|-----|---|---|
| 1   | 1 | 1 |
| 2   | 2 | 1 |
| 3   | 3 | 1 |
| 4   | 4 | 1 |
| 5   | 5 | 1 |

julia> DataFrame(a = 1, b = 1:5)
ERROR: New columns must have the same length as old columns
 in insert_single_column! at /home/tshort/.julia/v0.4/DataFrames/src/dataframe/dataframe.jl:309
 in setindex! at /home/tshort/.julia/v0.4/DataFrames/src/dataframe/dataframe.jl:368
 in DataFrame at /home/tshort/.julia/v0.4/DataFrames/src/dataframe/dataframe.jl:104

Here are results with this PR:

julia> DataFrame(a = 1, b = 1:5)
5x2 DataFrames.DataFrame
| Row | a | b |
|-----|---|---|
| 1   | 1 | 1 |
| 2   | 1 | 2 |
| 3   | 1 | 3 |
| 4   | 1 | 4 |
| 5   | 1 | 5 |

julia> DataFrame(a = 1:5, b = 1)
5x2 DataFrames.DataFrame
| Row | a | b |
|-----|---|---|
| 1   | 1 | 1 |
| 2   | 2 | 1 |
| 3   | 3 | 1 |
| 4   | 4 | 1 |
| 5   | 5 | 1 |

julia> DataFrame(a = 1:5, b = 1:3)
5x2 DataFrames.DataFrame
| Row | a | b |
|-----|---|---|
| 1   | 1 | 1 |
| 2   | 2 | 2 |
| 3   | 3 | 3 |
| 4   | 4 | 1 |
| 5   | 5 | 2 |

Note that this doesn't change the lack of recycling for other methods.

[nalimilan]

Makes sense, but I'd prefer making the recycling behaviour stricter: IMHO it should only work when the requested length is a multiple of the current length of the vector. It would thus be more similar to how broadcasting works. I've been bitten badly in R by silent recycling in cases where it was clearly a mistake, which would have been caught by a less tolerant policy.

[tshort]

I agree with that argument, @nalimilan. I'll update the PR tonight.

[tshort]

Any last comments? If not, I'll merge over the weekend.

[nalimilan]

I'd rather raise an error than return a vector of a different length than the requested one:

julia> rep_len([1, 2], 7)
6-element Array{Int64,1}:
 1
 2
 1
 2
 1
 2

[tshort]

Good call. Thx.

[nalimilan]

Do you still want to merge this?

[tshort]

No. After rethinking, I only want to handle the case of extending a single element. Ill redo it.

[tshort]

I've updated this to only recycle for a single element. If this is okay, I'll squash.

[johnmyleswhite]

That works for me.

[nalimilan]

This is length(x). Also, the description doesn't sound completely accurate: x is always recycled an integer number of times, so that the length is less than or equal to len. Though the behaviour you describe would be sensible (i.e. raise an error if not a multiple).

[tshort]

Thanks for catching. This is also out of date. I forgot to update the docs.

[nalimilan]

+1 too, modulo the small comment I added.

[tshort]

Squashed and ready to go, assuming that Travis doesn't complain.

[nalimilan]

Maybe better throw an ArgumentError? Also I would be really nice to give the name of the problematic column as well as its size vs. the expected one. :-)

[nalimilan]

You could also check that an error is thrown.

[ararslan]

Perhaps DimensionMismatch would be an appropriate exception here?

[nalimilan]

Bump. With the small fix I noted should be good to go.

[nalimilan]

Re-bump (see issue #973).

[tshort]

Sorry. Slammed until next weekend...
On May 22, 2016 11:46 AM, "Milan Bouchet-Valat" notifications@github.com
wrote:

Re-bump (see issue #973
#973).

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#882 (comment)

[nalimilan]

No worries, let's just no forget about this completely. :-)

[nalimilan]

Re-bump. :-)

[quinnj]

I think this has been resolved on master.