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
Fill gaps limited 7665 #9402
base: main
Are you sure you want to change the base?
Fill gaps limited 7665 #9402
Changes from 39 commits
e65bb4d
63dabc9
fdd3ca7
8393d72
e725008
d5466f5
1b8ea9e
b956e14
d717dd9
0c4fdab
7f06b3a
6090a4d
f8cc0c5
6de7640
07a0d01
274168c
e455f5b
d58b0ac
9700119
4a360fa
7389bf7
93c72f5
72c76db
6631aeb
7102381
5fe9feb
a74a840
3b7d6bc
9a5bc29
511323d
2463ded
ee34faf
ec221dd
41f2bf8
2c0d375
9ae4b26
5cdc223
078e546
e79f90f
e560701
281ba6d
fd04d54
94c88e3
046a592
ca4547d
76424b1
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.
Any thoughts from others on the naming? Would
.fill
be insufficiently specific that it's fillingna
? Wouldfill_missing
be clearer thanfill_gaps
?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.
Open to any of those.
.fill
sounds very concise, but maybe this is easily confused with.ffill
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
.fill
could be quite nice, do others have a view?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.
Maybe
gap_filler
instead, since this method does not actually fill the gaps.I'm also wondering if its better to have a method that constructs the appropriate mask that can be used later
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.
Good points! Just to explain:
gap_filler
emphasizes the returned object type nicely! However, I choosefill_gaps
because it fits the naming scheme of other object-returning functions better (e.g.rolling
andcoarsen
are not calledroller
andcoarser
in xarray, even though the operation is not perfomed immediately and an object is returned).Ultimately (I am a non-native english speaker) I am happy for any recommendations regarding nomenclature.
If you prefer
gap_filler
, I will change accordingly.The function API is also presented as an alternative in the initial proposal. I decided to go for the object way because it is shorter (one line) and less error prone (you might easily forget the
~
). If the mask is required, you can easily get it from the object: