-
Notifications
You must be signed in to change notification settings - Fork 55
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
Prepare package for Julia v1.7 #302
Conversation
Weird Error in nightly builds for Linux and macOS, but Windows is OK, so everything seems fine. |
@@ -458,7 +458,7 @@ function read_vals_default(obj::JldDataset, dtype::HDF5.Datatype, T::Type, dspac | |||
dsel_id::HDF5.hid_t, dims::Tuple{Vararg{Int}}) | |||
out = Array{T}(undef, dims) | |||
# Empty objects don't need to be read at all | |||
T.size == 0 && !T.mutable && return out | |||
T.size == 0 && !((VERSION >= v"1.7.0-DEV.1279") ? (Base.ismutabletype(T)) : (T.mutable)) && return out |
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.
Better to define
if isdefined(Base, :ismutabletype)
ismutabletype(T) = Base.ismutabletype(T)
else
ismutabletype(T) = T.mutable
end
and then call ismutabletype(T)
. That way you don't have a runtime VERSION
check, and you only have to put the check in one place.
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.
(See also JuliaLang/Compat.jl#759)
Let's get the Compat PR JuliaLang/Compat.jl#759 merged first and then rework this to use Compat. |
FYI @nandoconde, I officially retired from maintaining this package a long time ago, so long ago that I can't even remember where I posted the announcement. (I started too many 😄.) Likewise, Simon is on to other things. https://github.com/orgs/JuliaIO/people has a list of potential alternatives. |
Thank you both for the review!! As soon as JuliaLang/Compat.jl#759 is merged, I will use it to improve this one. |
It would be good to get this done now that 1.7 is out. |
fixed by #303 |
Change
T.mutable
forismutabletype