Skip to content
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

Improve docs #103

Merged
merged 3 commits into from
Jan 25, 2016
Merged

Improve docs #103

merged 3 commits into from
Jan 25, 2016

Conversation

tomasaschan
Copy link
Contributor

This builds on #99, so once that's merged, this should only list the last two commits.

Replicated much of @spencerlyon2's wonderful docstrings from Cubic for lower-order b-splines as well as fixed some things in the originals (mostly to make it easier to tell them all apart when listing e.g. help?> Interpolations.prefiltering_system).

I also found basically that all the information that was in the LaTeX document I wrote long ago is now in docstrings instead, so I removed it. (It's still in the history, if one wants to get it back.)

I haven't touched documentation for any non-B-spline interpolation code.

@tomasaschan
Copy link
Contributor Author

ref #46

function prefiltering_system{T,TC,GT<:GridType}(::Type{T}, ::Type{TC}, n::Int,
::Type{Cubic{Line}}, ::Type{GT})
prefiltering_system(T, TC, n, Quadratic{Line}, OnGrid)
function prefiltering_system{T,TC}(::Type{T}, ::Type{TC}, n::Int,
Copy link
Member

Choose a reason for hiding this comment

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

Did we decide not to rely on the Quadratic{Line}, OnGrid implementation here? and for Free below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I realized when documenting this that the systems aren't the same after all. prefiltering_system relies on inner_system_diags, which will yield different coefficients for quadratic and cubic (of course), and we lose that information if we just forward the call.

Copy link
Member

Choose a reason for hiding this comment

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

Oh man ok. Good catch. Sounds good, it's nice to have it below he description also

@sglyon
Copy link
Member

sglyon commented Jan 21, 2016

Excellent pull request! I love the docstrings. I think this will lower the bar to mostly metaprogramming stuff for people who want to understand how Interpolations.jl works by reading the source.

Constant b-splines are *nearest-neighbor* interpolations, and effectively
return `A[round(Int,x)]` when interpolating

Also, although the implementation is slightly different in order to re-use
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a copy-paste mistake; this remark is about linear, not constant, B-splines.

sglyon added a commit to JuliaLinearAlgebra/WoodburyMatrices.jl that referenced this pull request Jan 21, 2016
@@ -78,3 +78,35 @@ function prefilter!{TWeights,TCoefs<:AbstractArray,IT<:Tuple{Vararg{Union{BSplin
end

prefiltering_system(::Any, ::Any, ::Any, ::Any, ::Any) = nothing, nothing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timholy is this method actually good for anything? It seems to me that it would be better to get MethodErrors on prefiltering_system than on whatever happens to use it...

tomasaschan pushed a commit that referenced this pull request Jan 25, 2016
@tomasaschan tomasaschan merged commit b11bdd5 into master Jan 25, 2016
@tomasaschan tomasaschan deleted the improve-docs branch January 25, 2016 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants