Fixed deprecated functions (bytestring, is, symbol)#48
Fixed deprecated functions (bytestring, is, symbol)#48wookay wants to merge 3 commits intoJuliaDatabases:masterfrom
Conversation
… symbol to Symbol - Compat.ASCIIString for pgtype, pgdata
Codecov Report
@@ Coverage Diff @@
## master #48 +/- ##
=======================================
Coverage 75.56% 75.56%
=======================================
Files 3 3
Lines 221 221
=======================================
Hits 167 167
Misses 54 54Continue to review full report at Codecov.
|
iamed2
left a comment
iamed2
left a comment
There was a problem hiding this comment.
Two comments; looks good otherwise, thanks!
src/types.jl
Outdated
| if VERSION < v"0.5-dev+4194" | ||
| import Compat | ||
|
|
||
| function pgtype(t::Type) |
There was a problem hiding this comment.
Rather than an if, these can just be three methods of pgtype (e.g. pgtype{T<:Compat.ASCIIString}(::Type{T})
| ptr = storestring!(ptr, string(data)) | ||
| end | ||
|
|
||
| function pgdata(::PGStringTypes, ptr::Ptr{UInt8}, data::ByteString) |
There was a problem hiding this comment.
These functions were here so that ByteStrings could be stored directly and strings which don't have a unsafe_convert(Ptr{UInt8}, str) method would be converted to ByteStrings first. I think the correct way to fix these functions is to have this first one accept String and have the other ones call String(data) first.
There was a problem hiding this comment.
well, pgdata functions have always call the function storestring!.
and unsafe_convert(Ptr{UInt8}, str) has been always called in the function storestring!.
I need some test cases for this to investigate more.
thanks.
There was a problem hiding this comment.
The difference is that bytestring converted an AbstractString to a ByteString in all the functions that took an AbstractString, which ensured that the unsafe_convert call would succeed. It could pass ByteString directly through because it is backed by a null-terminated cstr, so unsafe_convert will succeed. Some T<:AbstractString don't have that backing, and need to be converted to a String first before they can be passed to storestring!.
wookay
force-pushed
the
deprecated_bytestring_is
branch
from
2285635 to
e87b3a5
Compare
* updates for Julia v0.5 compatibility (mostly related to Strings warnings) * added missing types definition + updated deprecated * removed io seek unused functions + duplicate function * Removed duplicate function declarations * Fixed type alias declaration * Changed bytestring to unsafe_string * Fixed String to be AbstractString * Fixed convert function redefinition warning * Fixed multiple type declarations * fixed getting byte array from strings * fixed deprecated bytestring in test * Fixed formatting * added abstact type for julia 0.6 (with compat support)
| ptr = storestring!(ptr, string(data)) | ||
| end | ||
|
|
||
| function pgdata(::PGStringTypes, ptr::Ptr{UInt8}, data::ByteString) |
There was a problem hiding this comment.
The difference is that bytestring converted an AbstractString to a ByteString in all the functions that took an AbstractString, which ensured that the unsafe_convert call would succeed. It could pass ByteString directly through because it is backed by a null-terminated cstr, so unsafe_convert will succeed. Some T<:AbstractString don't have that backing, and need to be converted to a String first before they can be passed to storestring!.
5 participants
Hello
bytestringtounsafe_string,isto===,symboltoSymbolCompat.ASCIIStringforpgtype,pgdatathanks.