Skip to content

Commit

Permalink
Avoid override of getproperty.
Browse files Browse the repository at this point in the history
  • Loading branch information
orenbenkiki committed Jun 19, 2024
1 parent 1a6a2f8 commit 56f13cc
Show file tree
Hide file tree
Showing 18 changed files with 79 additions and 93 deletions.
2 changes: 1 addition & 1 deletion docs/v0.1.0/.documenter-siteinfo.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"documenter":{"julia_version":"1.10.4","generation_timestamp":"2024-06-19T12:48:58","documenter_version":"1.4.1"}}
{"documenter":{"julia_version":"1.10.4","generation_timestamp":"2024-06-19T14:10:28","documenter_version":"1.4.1"}}
4 changes: 3 additions & 1 deletion docs/v0.1.0/formats.html
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,8 @@ <h2 id="Read-API">
</code> storage formats.
</p>
<p>We require each storage format to have a
<code>.name
</code> and an
<code>.internal::
</code>
<a href="formats.html#Daf.Formats.Internal">
Expand Down Expand Up @@ -477,7 +479,7 @@ <h2 id="Read-API">
<section>
<div>
<pre>
<code class="language-julia hljs">Internal(name::AbstractString)
<code class="language-julia hljs">struct Internal ... end
</code>
</pre>
<p>Internal data we need to keep in any concrete
Expand Down
10 changes: 6 additions & 4 deletions docs/v0.1.0/messages.html
Original file line number Diff line number Diff line change
Expand Up @@ -265,16 +265,18 @@ <h1 id="Messages">
<section>
<div>
<pre>
<code class="language-julia hljs">unique_name(prefix::AbstractString)::AbstractString
<code class="language-julia hljs">unique_name(prefix::AbstractString, separator::AbstractString = &quot;#&quot;)::AbstractString
</code>
</pre>
<p>Using short, human-readable unique names for things is a great help when debugging. Normally one has to choose between using a human-provided short non-unique name, and an opaque object identifier, or a combination thereof. This function replaces the opaque object identifier with a short counter, which gives names that are both unique and short.
</p>
<p>That is, this will return a unique name starting with the
<code>prefix
</code> and followed by
</code> and followed by the
<code>separator
</code> (by default,
<code>#
</code>, the process index (if using multiple processes), and an index (how many times this name was used in the process). For example,
</code>), the process index (if using multiple processes), and an index (how many times this name was used in the process). For example,
<code>unique_name(&quot;foo&quot;)
</code> will return
<code>foo
Expand All @@ -290,7 +292,7 @@ <h1 id="Messages">
</p>
<p>To help with tests, if the
<code>prefix
</code> ends with
</code> contains
<code>!
</code>, we return it as-is, accepting it may not be unique.
</p>
Expand Down
2 changes: 1 addition & 1 deletion docs/v0.1.0/search_index.js

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion src/anndata_format.jl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ using Muon
using SparseArrays

import ..Formats
import ..Formats.Internal
import ..Readers.require_matrix

"""
Expand Down
25 changes: 15 additions & 10 deletions src/chains.jl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ override earlier data sets. However, if an axis exists in more than one data set
identical. This isn't typically created manually; instead call [`chain_reader`](@ref).
"""
struct ReadOnlyChain <: DafReadOnly
name::AbstractString
internal::Internal
dafs::Vector{DafReader}
end
Expand All @@ -50,6 +51,7 @@ exists only in this writer. That is, it is impossible to delete from a chain som
readers; it is only possible to override it.
"""
struct WriteChain <: DafWriter
name::AbstractString
internal::Internal
dafs::Vector{DafReader}
daf::DafWriter
Expand Down Expand Up @@ -81,9 +83,10 @@ function chain_reader(dafs::AbstractVector{<:DafReader}; name::Maybe{AbstractStr
name = join([daf.name for daf in dafs], ";")
@assert name !== nothing
end
name = unique_name(name, ";#")

internal_dafs = reader_internal_dafs(dafs, name)
chain = ReadOnlyChain(Internal(name; is_frozen = true), internal_dafs)
chain = ReadOnlyChain(name, Internal(; is_frozen = true), internal_dafs)
@debug "Daf: $(depict(chain)) chain: $(join([daf.name for daf in dafs], ";"))"
return chain
end
Expand Down Expand Up @@ -115,11 +118,13 @@ function chain_writer(dafs::AbstractVector{<:DafReader}; name::Maybe{AbstractStr
end
name = join([daf.name for daf in dafs], ";")
@assert name !== nothing
else
name = unique_name(name)
end

internal_dafs = reader_internal_dafs(dafs, name)
reader = ReadOnlyChain(Internal(name; is_frozen = false), internal_dafs)
chain = WriteChain(reader.internal, reader.dafs, dafs[end])
reader = ReadOnlyChain(name, Internal(; is_frozen = false), internal_dafs)
chain = WriteChain(name, reader.internal, reader.dafs, dafs[end])
@debug "Daf: $(depict(chain)) chain: $(join([daf.name for daf in dafs], ";"))"
return chain
end
Expand Down Expand Up @@ -245,7 +250,7 @@ function Formats.format_delete_scalar!(chain::WriteChain, name::AbstractString;
error(
"failed to delete the scalar: $(name)\n" *
"from the daf data: $(chain.daf.name)\n" *
"of the chain: $(chain.name)\n" * # NOLINT
"of the chain: $(chain.name)\n" *
"because it exists in the earlier: $(daf.name)",
)
end
Expand Down Expand Up @@ -306,7 +311,7 @@ function Formats.format_delete_axis!(chain::WriteChain, axis::AbstractString)::N
error(
"failed to delete the axis: $(axis)\n" *
"from the daf data: $(chain.daf.name)\n" *
"of the chain: $(chain.name)\n" * # NOLINT
"of the chain: $(chain.name)\n" *
"because it exists in the earlier: $(daf.name)",
)
end
Expand Down Expand Up @@ -418,7 +423,7 @@ function Formats.format_delete_vector!(
"failed to delete the vector: $(name)\n" *
"of the axis: $(axis)\n" *
"from the daf data: $(chain.daf.name)\n" *
"of the chain: $(chain.name)\n" * # NOLINT
"of the chain: $(chain.name)\n" *
"because it exists in the earlier: $(daf.name)",
)
end
Expand Down Expand Up @@ -563,7 +568,7 @@ function Formats.format_delete_matrix!(
"for the rows axis: $(rows_axis)\n" *
"and the columns axis: $(columns_axis)\n" *
"from the daf data: $(chain.daf.name)\n" *
"of the chain: $(chain.name)\n" * # NOLINT
"of the chain: $(chain.name)\n" *
"because it exists in the earlier: $(daf.name)",
)
end
Expand Down Expand Up @@ -675,14 +680,14 @@ end

function Messages.depict(value::ReadOnlyChain; name::Maybe{AbstractString} = nothing)::String
if name === nothing
name = value.name # NOLINT
name = value.name
end
return "ReadOnly Chain $(name)"
end

function Messages.depict(value::WriteChain; name::Maybe{AbstractString} = nothing)::String
if name === nothing
name = value.name # NOLINT
name = value.name
end
return "Write Chain $(name)"
end
Expand All @@ -691,7 +696,7 @@ function ReadOnly.read_only(daf::ReadOnlyChain; name::Maybe{AbstractString} = no
if name === nothing
return daf
else
return ReadOnlyChain(Formats.renamed_internal(daf.internal, name), daf.dafs)
return ReadOnlyChain(name, daf.internal, daf.dafs)
end
end

Expand Down
7 changes: 4 additions & 3 deletions src/contracts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,12 @@ This isn't exported and isn't created manually; instead call [`contractor`](@ref
to avoid code duplication, it is still a [`DafWriter`](@ref) rather than a [`DafReader`](@ref).
"""
struct ContractDaf <: DafWriter
name::AbstractString
internal::Formats.Internal
computation::AbstractString
axes::Dict{AbstractString, Tracker}
data::Dict{DataKey, Tracker}
daf::DafReader
name::AbstractString
internal::Formats.Internal
overwrite::Bool
end

Expand All @@ -294,7 +294,8 @@ function contractor(
)::ContractDaf
axes = collect_axes(contract)
data = collect_data(contract, axes)
return ContractDaf(computation, axes, data, daf, daf.name, daf.internal, overwrite)
name = unique_name("$(daf.name).for.$(computation)")
return ContractDaf(name, daf.internal, computation, axes, data, daf, overwrite)
end

function collect_axes(contract::Contract)::Dict{AbstractString, Tracker}
Expand Down
6 changes: 4 additions & 2 deletions src/files_format.jl
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ The valid `mode` values are as follows (the default mode is `r`):
| `w` | Yes | Yes | Yes | [`FilesDaf`](@ref) |
"""
struct FilesDaf <: DafWriter
name::AbstractString
internal::Internal
path::AbstractString
mode::AbstractString
Expand Down Expand Up @@ -226,11 +227,12 @@ function FilesDaf(
name = path
end
end
name = unique_name(name)

if is_read_only
file = read_only(FilesDaf(Internal(name; is_frozen = true), path, mode, "r"))
file = read_only(FilesDaf(name, Internal(; is_frozen = true), path, mode, "r"))
else
file = FilesDaf(Internal(name; is_frozen = false), path, mode, "r+")
file = FilesDaf(name, Internal(; is_frozen = false), path, mode, "r+")
end
@debug "Daf: $(depict(file)) path: $(path)"
return file
Expand Down
22 changes: 4 additions & 18 deletions src/formats.jl
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ mutable struct WriterThread
end

"""
Internal(name::AbstractString)
struct Internal ... end
Internal data we need to keep in any concrete [`FormatReader`](@ref). This has to be available as a `.internal` data
member of the concrete format. This enables all the high-level [`DafReader`](@ref) and [`DafWriter`](@ref) functions.
Expand All @@ -120,7 +120,6 @@ The constructor will automatically call [`unique_name`](@ref) to try and make th
messages.
"""
struct Internal
name::AbstractString
cache::Dict{AbstractString, CacheEntry}
dependency_cache_keys::Dict{AbstractString, Set{AbstractString}}
version_counters::Dict{DataKey, UInt32}
Expand All @@ -129,9 +128,8 @@ struct Internal
is_frozen::Bool
end

function Internal(name::AbstractString; is_frozen::Bool)::Internal
function Internal(; is_frozen::Bool)::Internal
return Internal(
unique_name(name),
Dict{AbstractString, CacheEntry}(),
Dict{AbstractString, Set{AbstractString}}(),
Dict{DataKey, UInt32}(),
Expand All @@ -141,23 +139,11 @@ function Internal(name::AbstractString; is_frozen::Bool)::Internal
)
end

function renamed_internal(internal::Internal, name::AbstractString)::Internal
return Internal(
name,
internal.cache,
internal.dependency_cache_keys,
internal.version_counters,
internal.cache_lock,
internal.data_lock,
internal.is_frozen,
)
end

"""
An low-level abstract interface for reading from `Daf` storage formats.
We require each storage format to have a `.internal::`[`Internal`](@ref) property. This enables all the high-level
`DafReader` functions.
We require each storage format to have a `.name` and an `.internal::`[`Internal`](@ref) property. This enables all the
high-level `DafReader` functions.
Each storage format must implement the functions listed below for reading from the storage.
"""
Expand Down
4 changes: 3 additions & 1 deletion src/h5df_format.jl
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ The valid `mode` values are as follows (the default mode is `r`):
to maximize efficiency of mapped vectors and matrices, and the `w+` mode is converted to `cw`.
"""
struct H5df <: DafWriter
name::AbstractString
internal::Internal
root::Union{HDF5.File, HDF5.Group}
mode::AbstractString
Expand Down Expand Up @@ -214,8 +215,9 @@ function H5df(
name = root.filename
end
end
name = unique_name(name)

h5df = H5df(Internal(name; is_frozen = is_read_only), root, mode)
h5df = H5df(name, Internal(; is_frozen = is_read_only), root, mode)
@debug "Daf: $(depict(h5df)) root: $(root)"
if is_read_only
return read_only(h5df)
Expand Down
8 changes: 3 additions & 5 deletions src/memory_format.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,11 @@ object that just keeps references to the data it is given.
This is the "default" storage type you should use, unless you need to persist the data on the disk.
"""
struct MemoryDaf <: DafWriter
name::AbstractString
internal::Internal

scalars::Dict{AbstractString, StorageScalar}

axes::Dict{AbstractString, AbstractVector{<:AbstractString}}

vectors::Dict{AbstractString, Dict{AbstractString, StorageVector}}

matrices::Dict{AbstractString, Dict{AbstractString, Dict{AbstractString, StorageMatrix}}}
end

Expand All @@ -46,7 +43,8 @@ function MemoryDaf(; name::AbstractString = "memory")::MemoryDaf
axes = Dict{AbstractString, AbstractVector{<:AbstractString}}()
vectors = Dict{AbstractString, Dict{AbstractString, StorageVector}}()
matrices = Dict{AbstractString, Dict{AbstractString, Dict{AbstractString, StorageMatrix}}}()
memory = MemoryDaf(Internal(name; is_frozen = false), scalars, axes, vectors, matrices)
name = unique_name(name)
memory = MemoryDaf(name, Internal(; is_frozen = false), scalars, axes, vectors, matrices)
@debug "Daf: $(depict(memory))"
return memory
end
Expand Down
20 changes: 10 additions & 10 deletions src/messages.jl
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@ UNIQUE_NAME_PREFIXES = Dict{AbstractString, Int64}()
UNIQUE_NAME_LOCK = ReentrantLock()

"""
unique_name(prefix::AbstractString)::AbstractString
unique_name(prefix::AbstractString, separator::AbstractString = "#")::AbstractString
Using short, human-readable unique names for things is a great help when debugging. Normally one has to choose between
using a human-provided short non-unique name, and an opaque object identifier, or a combination thereof. This function
replaces the opaque object identifier with a short counter, which gives names that are both unique and short.
That is, this will return a unique name starting with the `prefix` and followed by `#`, the process index (if using
multiple processes), and an index (how many times this name was used in the process). For example, `unique_name("foo")`
will return `foo` for the first usage, `foo#2` for the 2nd, etc. If using multiple processes, it will return `foo`,
`foo#1.2`, etc.
That is, this will return a unique name starting with the `prefix` and followed by the `separator` (by default, `#`),
the process index (if using multiple processes), and an index (how many times this name was used in the process). For
example, `unique_name("foo")` will return `foo` for the first usage, `foo#2` for the 2nd, etc. If using multiple
processes, it will return `foo`, `foo#1.2`, etc.
That is, for code where the names are unique (e.g., a simple script or Jupyter notebook), this doesn't mess up the
names. It only appends a suffix to the names if it is needed to disambiguate between multiple uses of the same name.
To help with tests, if the `prefix` ends with `!`, we return it as-is, accepting it may not be unique.
To help with tests, if the `prefix` contains `!`, we return it as-is, accepting it may not be unique.
"""
function unique_name(prefix::AbstractString)::AbstractString
if prefix[end] == '!'
function unique_name(prefix::AbstractString, separator::AbstractString = "#")::AbstractString
if contains(prefix, '!')
return String(prefix)
end

Expand All @@ -53,9 +53,9 @@ function unique_name(prefix::AbstractString)::AbstractString
if counter == 1
return prefix
elseif nprocs() > 1
return "$(prefix)#$(myid()).$(counter)" # untested
return "$(prefix)$(separator)$(myid()).$(counter)" # untested
else
return "$(prefix)#$(counter)"
return "$(prefix)$(separator)$(counter)"
end
end

Expand Down
Loading

0 comments on commit 56f13cc

Please sign in to comment.