Fix breakage caused by JuliaLang/julia/pull/53896 #1133
Conversation
|
|
||
| if isdefined(Base,:wrap) | ||
| __wrap(x,pos) = Base.wrap(Array,x,pos) | ||
| if isdefined(Base, :Memory) |
There was a problem hiding this comment.
can't you get rid of this entire function and just make it use view
There was a problem hiding this comment.
i want view(x::Array) return x, and view(x::Memory) does not work
There was a problem hiding this comment.
or do you mean in the function getbytebuffer itself?
There was a problem hiding this comment.
yeah, this function only has one use and now that view isn't a new name from base you can just use it without the check.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1133 +/- ##
==========================================
+ Coverage 90.12% 90.41% +0.29%
==========================================
Files 9 9
Lines 2309 2338 +29
==========================================
+ Hits 2081 2114 +33
+ Misses 228 224 -4 ☔ View full report in Codecov by Sentry. |
|
I'd like to go ahead and merge this to fix the regression on master. If there's additional simplification to be had, let's do a follow up PR. |
|
|
||
| if isdefined(Base,:wrap) | ||
| __wrap(x,pos) = Base.wrap(Array,x,pos) | ||
| if isdefined(Base, :Memory) |
There was a problem hiding this comment.
Julia 1.11 also has Base.Memory (but not the recent wrap rename) - Will this PR be backwards compatible?
There was a problem hiding this comment.
the wrap->view will be back-ported to 1.11
There was a problem hiding this comment.
It seems rude to break people on the current RC. Can we put in appropriate checks?
There was a problem hiding this comment.
I can add an extra if for the case that Memory and wrap is defined
|
I'm merging to fix the breakage - and perhaps further improvements can follow. |
5 participants
JuliaLang/julia#53896 removed
Base.wrap, and i did use that to fix CSV in the last patch. This now usesview(x::Memory,len)instead.fixes #1132