Fix breakage caused by JuliaLang/julia/pull/53896 #1133
Conversation
| @@ -245,10 +245,11 @@ end | |||
| # one-liner suggested from ScottPJones | |||
| consumeBOM(buf, pos) = (length(buf) >= 3 && buf[pos] == 0xef && buf[pos + 1] == 0xbb && buf[pos + 2] == 0xbf) ? pos + 3 : pos | |||
|
|
|||
| 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. |
| @@ -245,10 +245,11 @@ end | |||
| # one-liner suggested from ScottPJones | |||
| consumeBOM(buf, pos) = (length(buf) >= 3 && buf[pos] == 0xef && buf[pos + 1] == 0xbb && buf[pos + 2] == 0xbf) ? pos + 3 : pos | |||
|
|
|||
| 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. |
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