Skip to content
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

add propertynames #139

Merged
merged 1 commit into from
Nov 16, 2021
Merged

add propertynames #139

merged 1 commit into from
Nov 16, 2021

Conversation

ericphanson
Copy link
Contributor

Closes #131

Here, I define the "public" properties as the getproperty overloads, and the private properties as any remaining internal fields. My thinking is then downstream types can accept this or define their own propertynames to declare which fields whey want to be private or not.

I used Base.merge_names here; this is a Julia internal function so perhaps we should not depend on it. It has at least existed for 4 years (https://github.com/JuliaLang/julia/blame/ff001a4e1f5b7f949f5a2328520a7e78bc1957b4/base/namedtuple.jl#L211). The reason I wanted to use it is to have a fast way of merging the tuples of symbols without duplicates (a slow alternative would be Tuple(unique((public_names..., fieldnames(T)...)))).

@nickrobinson251
Copy link
Contributor

I used Base.merge_names here... to have a fast way of merging the tuples of symbols without duplicates

I think this is the same motivation as for the hack NamedDims.jl uses to make operations on tuples of symbols non-allocating
https://github.com/invenia/NamedDims.jl/blob/77b09418820bca221d5bff68eacd613a8bfac6ed/src/name_core.jl#L10-L30
Anyway, using Base.merge_names seems fine to me. But then again, i suspect propertynames(x; private=true) being slow/allocating seems fine too.

@rofinn rofinn merged commit eb9895e into rofinn:master Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is e.g. path.separator part of the public API?
3 participants