Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Flexible vcat #1659
Flexible vcat #1659
Changes from 8 commits
90d4cf4
ad9b3b7
8300c28
1517282
d9070e6
a52f4c7
ae19b61
0bb967c
564087c
50f5ae1
907774c
2d99d94
20f9bbd
791edd5
e6fd17c
3c4a272
54998d4
406ba08
71fd8a4
b7fe9f3
af102be
b525ec2
d81efed
9bde8fb
1ad6e54
44113ae
802180c
e9ade20
6790c63
d64898f
dc591b2
73a676d
861dd18
b1be144
4f8b662
506a3a4
45d8443
d2b4e11
4ccdda1
14522e1
ed5ed6f
c25ac46
2d0d831
8bc28b4
5a3a6ca
6aa7aa8
a23dc63
01ad9ab
451ca2a
8b0b7c8
6f7d1b5
0e324bb
e34451d
6b24f75
a42e44a
c8cd4b3
d8324a2
1d6b01e
6af37c8
9e8ebe2
bc6220e
24be56a
8ce9839
bc07df5
f2ccec0
f0d3a7a
6e5fba5
3ab3816
995eb94
2643142
187f55d
3d6c4d1
12ffd5f
bcdf155
e1c3c9e
7459b1d
0805b19
c947b7a
b1c777d
c26839e
671bba8
9661f76
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This argument isn't used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not looked at the PR in detail - but it seems you re-introduce some old code that was already removed here (at least in the comments)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I had missed the notification.
RepeatedVector
can probably help here (if using a generator isn't enough).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh why did I not think of a generator. It's fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that a generator doesn't infer eltype correclty. Columns are of type
Any
rather thanUnion{T, Missing}
.RepeatedVector
doesn't work because it's not defined at this point in the code, so we can't use it here. (Should we have a place for all struct defintions?)Let me know what to do next. I think there is an issue for this generator issue but I can't find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Maybe with
Iterators.repeated
? (eltype
won't be defined for generators since it needs inference.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for
SubDataFrame
? Thendf[name]
should be enough and you can drop the branch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this will work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do not need that branch. Anyway
data
will be type unstable I think (but probably it is best to check it).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting rid of the branch works fine.