You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.
It looks like you are trying to ensure that the labels are unique in order to use them as primary keys somewhere. I think the better options are:
Use the positional ID which is guaranteed to be unique. There is always exactly 1 "n-th state" or "n-th transition". This satisfies your need for primary keys. It is actually better than guaranteeing that the ID is unique. The parts(X, :A) are always contiguously indexed in 1:nparts(X,:A).
You are storing a tuple where the last element of the tuple is Symbol(""), this is probably going to bite you later since you will have elements of a tuple that don't mean anything. It would be better to make the label (X, i) rather than (Xi, ""). In general it is bad to pack structured data like "a pair with a symbol then a number" into a string. You are much better off representing that as Tuple{String, Int}.
The text was updated successfully, but these errors were encountered:
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
I was just browsing the code and I noticed this function:
model-service/src/model-transform/stratification.jl
Line 104 in fac5ed2
It looks like you are trying to ensure that the labels are unique in order to use them as primary keys somewhere. I think the better options are:
parts(X, :A)
are always contiguously indexed in1:nparts(X,:A)
.Symbol("")
, this is probably going to bite you later since you will have elements of a tuple that don't mean anything. It would be better to make the label(X, i)
rather than(Xi, "")
. In general it is bad to pack structured data like "a pair with a symbol then a number" into a string. You are much better off representing that asTuple{String, Int}
.The text was updated successfully, but these errors were encountered: