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

padding/enlargement improvements #12

Closed
wants to merge 3 commits into from
Closed

padding/enlargement improvements #12

wants to merge 3 commits into from

Conversation

behinger
Copy link
Collaborator

@behinger behinger commented Jul 20, 2022

  • allow pad_value to be a function (e.g. mean)
  • rename protected 'padding' word to enlarge

Helps to investigate #9, also padding=0 did not work prior

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

Codecov Report

Merging #12 (d8bfb3f) into master (b2f8b98) will decrease coverage by 0.66%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master      #12      +/-   ##
==========================================
- Coverage   90.83%   90.16%   -0.67%     
==========================================
  Files           4        4              
  Lines         120      122       +2     
==========================================
+ Hits          109      110       +1     
- Misses         11       12       +1     
Impacted Files Coverage Δ
src/core-recipe.jl 85.29% <75.00%> (-1.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2f8b98...d8bfb3f. Read the comment docs.


padded_data = lift(pad_data, p.data, npositions, p.pad_value)

if !isempty(methods(to_value(p.enlarge_value)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

calling methods explicitly is a bit weird. What do you want to do here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

find out whether elarge_value is a function or not. This is what I googled how to find out whether a variable is callable or not. I guess, one could also check whether it is a Number, and else call the function. probably easier to understand

@palday palday mentioned this pull request Jul 27, 2022
@SimonDanisch SimonDanisch mentioned this pull request Aug 17, 2022
@palday
Copy link
Collaborator

palday commented Aug 30, 2022

Seems to be superseded by #20. If there is still something good here, then we need to merge master in this branch after #20 lands.

@SimonDanisch
Copy link
Member

I don't think there is...If we want to use the mean for extrapolation, we'd need a new PR with a new extrapolator! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants