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
Materialize
DimArray
orDimStack
From a Table #739base: main
Are you sure you want to change the base?
Materialize
DimArray
orDimStack
From a Table #739Changes from 27 commits
60256a0
3526b96
eab2fa0
d4892df
ea6751a
13c80da
6a9d26e
9164c22
2ebec1c
8e791bf
4cd5f9d
0c1991a
8758ba9
4534de5
119fa30
ed395ca
00336af
532f887
c98dcb0
06a2c91
3bacf33
4ced6f7
c846dfd
fe2c871
61f8220
3d28b43
dbe7b99
f410988
a17f069
9bdded9
5451087
faf4d76
02f60a3
fafd357
d7f15f5
34a0a69
d0b9eb7
32b0c00
0ea72a0
bc62932
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.
We should probably do a
Tables.istable
check at this point for a nicer error and stack trace, rather than failing deep in the internals.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.
Again we probably need a
Tables.istable
check hereThere 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.
Should this be
AbstractVector{Union{Number,DateTime}
?I'm wondering what happens to strings, symbols and other objects that need to go in
Categorical
lookups.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 should work with almost any type. If the coordinates are non-numerical, then we will internally dispatch on the following methods:
_unique_vals
will just return all unique values, where_round_dim_val
is just the identity function for non-numerical coordinates._guess_dim_order
should work for anything that can be sorted, which is the case for bothString
andSymbol
._guess_dim_span
will returnDD.Irregular()
for non-numerical coordinates.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.
We used to need a try catch for
issorted
in case<
is not defined for a typeThere 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.
We can also find regular spans for Dates?
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 should let us handle data that can't be sorted:
And this should retrieve the span from
Date
andDateTime
objects:However, there seems to be a problem with constructing a
LinRange
fromDate
objects:Thus, I'm not sure how we should construct a
Dimension
with regularly spaced dates.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.
A StepRange works for Dates. Probably we should use StepRangeLen instead of LinRange where possible anyway
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.
That works. Do you want to use
StepRangeLen
for numerical coordinates or just dates?Here's the result:
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's probably better for everything, except a bit slower. It didn't exist when I first wrote this package and uses of LinRange are just legacy from that.
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.
Are all of these DD and DimensionalData qualifiers needed?