Skip to content

Conversation

lkdvos
Copy link
Member

@lkdvos lkdvos commented May 25, 2024

This PR simplifies the usage of conj flags by replacing the symbols with booleans. This is especially useful in AD, where comparing and negating with booleans is more convenient.

Importantly, as a Bool is a subtype of Number, this introduces some method ambiguities for the functions syntax, which is resolved by restricting the types of the labels to be a Tuple or AbstractVector of Int, Symbol or Char.

Alias for supported label containers.
"""
const Labels{I<:LabelType} = Union{Tuple{Vararg{I}},AbstractVector{I}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This excludes things like [1,3,:x,:a] but I guess that is acceptable. We can always recommend tuples, as (1,3,:x,'a') isa Labels is still true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collect(LabelType, [1, 2, :x, :a]) should also still work in that case :)

@Jutho
Copy link
Member

Jutho commented Jun 11, 2024

While I do think that code clarity is slightly reduced here and there compared to the explicit use of N and C (as Char or as Symbol), I do acknowledge the code simplification that it induces.

@lkdvos lkdvos merged commit 65cad25 into master Jun 11, 2024
@lkdvos lkdvos deleted the ld-conjflag branch June 11, 2024 15:35
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.

2 participants