Started DimensionalDataInterpolationsExt.jl for AbstractDimVectors#1091
Started DimensionalDataInterpolationsExt.jl for AbstractDimVectors#1091kapple19 wants to merge 9 commits into
DimensionalDataInterpolationsExt.jl for AbstractDimVectors#1091Conversation
498e78c to
aa5a061
Compare
|
I don't know how to stop the other CI runs before my latest force-push 😭 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1091 +/- ##
==========================================
+ Coverage 86.96% 87.00% +0.03%
==========================================
Files 61 63 +2
Lines 5848 5871 +23
==========================================
+ Hits 5086 5108 +22
- Misses 762 763 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| """ | ||
| ``` | ||
| (::DataInterpolations.AbstractInterpolation)( |
There was a problem hiding this comment.
| (::DataInterpolations.AbstractInterpolation)( | |
| (::Type{<: DataInterpolations.AbstractInterpolation})( |
There was a problem hiding this comment.
I would actually document each one separately, if you are running this in an @eval loop.
| function DataInterpolations.QuadraticInterpolation( | ||
| data::AbstractDimVector, | ||
| mode::Symbol, | ||
| args...; | ||
| kw... | ||
| ) | ||
| return QuadraticInterpolation( | ||
| data |> parent, | ||
| dims(data) |> only |> parent |> parent, | ||
| mode, | ||
| args...; | ||
| kw... | ||
| ) | ||
| end |
There was a problem hiding this comment.
If these are all the same, I would recommend defining them in an @eval loop for simplicity and ease of editing
There was a problem hiding this comment.
Unfortunately, they're not all the same. Some require or have options for one or more arguments.
Additionally, one of them isa Function and not a struct.
| DimensionalDataAlgebraOfGraphicsExt = "AlgebraOfGraphics" | ||
| DimensionalDataCategoricalArraysExt = "CategoricalArrays" | ||
| DimensionalDataDiskArraysExt = "DiskArrays" | ||
| DimensionalDataInterpolationsExt = "DataInterpolations" |
There was a problem hiding this comment.
| DimensionalDataInterpolationsExt = "DataInterpolations" | |
| DimensionalDataDataInterpolationsExt = "DataInterpolations" |
Two datas in a row, I know, but this makes it consistent with the rest of the extensions (and I think there was also an Interpolations.jl extension PR)
There was a problem hiding this comment.
What about the discussed solution of brevity where, Julia already displays the base package in addition to the extension, so we should just be naming it DataInterpolationsExt.jl?
There was a problem hiding this comment.
Have to agree with @asinghvi17, just go with the standard. We actually messed it up in the past and I mean to standardise the rest too.
rafaqz
left a comment
rafaqz
left a comment
There was a problem hiding this comment.
Docs in tabs are great.
And this is a good, straightforward place to start
|
|
||
| # Interpolation of `DimensionalData.jl` Objects | ||
|
|
||
| The following functionalities are experimental and under development. |
There was a problem hiding this comment.
Semver and breakage is the key thing to mention here, otherwise it's not clear what this means practically.
| ) | ||
| return Itp( | ||
| data |> parent, | ||
| dims(data) |> only |> parent |> parent, |
There was a problem hiding this comment.
We should think about Points vs Intervals here, may need to use shiftlocus(Center(), dim) to standardise everything (the lookup values can also be the Start() of the interval)
Also, this line can be an internal function we use for all the constructors.
There was a problem hiding this comment.
Also, this line can be an internal function we use for all the constructors.
Not really, different interpolation packages have different interfaces.
Unless I've understood you incorrectly.
There was a problem hiding this comment.
may need to use
shiftlocus
I need an explanation/demonstration for this.
| For the following examples, the following `DimArray` is used: | ||
|
|
||
| ```@ansi Interpolation1D | ||
| y = DimArray( |
There was a problem hiding this comment.
I would use a name besides y, as it suggests a Y dimension. da is an obvious one, or just A
|
Thanks a tonne guys! |
|
Take 2. Critiques please 😁 |
|
Uh, should I rebase it? If so, where? |
|
You can rebase main if you want |
|
@kapple19 it turns out I need this for work so I'm going to have a go at DimInterpolator objects and stacks of them based on DataInterpolations.jl |
|
Sounds good. |
|
No problem, I haven't had much time anyway |
|
@rafaqz shall I close this PR given: #1126 (comment)? |
3 participants
First pass at interpolation wrappers.
Critiques please 😊