-
Notifications
You must be signed in to change notification settings - Fork 49
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
add BorderArray #99
add BorderArray #99
Conversation
Codecov Report
@@ Coverage Diff @@
## master #99 +/- ##
==========================================
+ Coverage 81.12% 81.44% +0.32%
==========================================
Files 8 9 +1
Lines 1118 1159 +41
==========================================
+ Hits 907 944 +37
- Misses 211 215 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #99 +/- ##
=========================================
+ Coverage 81.12% 81.43% +0.3%
=========================================
Files 8 9 +1
Lines 1118 1174 +56
=========================================
+ Hits 907 956 +49
- Misses 211 218 +7
Continue to review full report at Codecov.
|
@timholy what do you think about the general design of
|
This looks superficially awesome, but I haven't had bandwidth recently. Maybe I can review this tomorrow, sorry for the delay. |
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.
Very nice, as usual! Only very minor points here.
The implementation of getindex
via constructing the CartesianIndex
is a bit unconventional, but here it makes sense. Some day we may want to allow different border types along different dimensions (see #94), in which case we'll need an index-by-index implementation, but that's not really your problem here.
Co-Authored-By: jw3126 <[email protected]>
Once this passes tests, I approve merging. Thanks for this, fantastic work! |
@timholy can we tag a new release? |
Sorry I didn't notice the question about tagging. Absolutely! There's nothing breaking here, right? So this could be 0.6.1? I think you should be able tag yourself, if you like. Perhaps give it a try, just so we know whether you are as empowered as you deserve to be. |
Yeah, there is no breaking change, at least no intended one 😄. I will see if I can create a release. |
#98