Skip to content

nasty edge cases with prefix and unprefix #122

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

Open
penelopeysm opened this issue May 5, 2025 · 2 comments
Open

nasty edge cases with prefix and unprefix #122

penelopeysm opened this issue May 5, 2025 · 2 comments

Comments

@penelopeysm
Copy link
Member

penelopeysm commented May 5, 2025

julia> vnp = prefix(@varname(p), @varname(s[1].a))
s[1].a.p

julia> subsumes(@varname(s[1].a), vnp)
false

julia> vnp == @varname(s[1].a.p)
false

This arises because of the associativity of ComposedFunction -- for example in s[1].a.p the lens can be either ComposedFunction(ComposedFunction(.p, .a), .[1])) or it could be ComposedFunction(.p, ComposedFunction(.a, .[1])) (full functions not shown for simplicity).

There are two ways to handle this. One is to carefully disambiguate all cases with lots of if/else or multiple dispatch methods. The other is to enforce normalisation such that ComposedFunction can only have ComposedFunction as its outer optic (for example) and not as its inner optic. That would require us to either write our own ComposedFunction or upstream it.

@mhauru
Copy link
Member

mhauru commented Jun 2, 2025

Could we do the normalisation in a VarName inner constructor?

@penelopeysm
Copy link
Member Author

penelopeysm commented Jun 2, 2025

We could indeed, the only downside would be that would happen every time we construct a VarName. I think I originally only intended to do it on the prefix / unprefix functions (because I think that's the only place where this can happen), but maybe that's a micro-optimisation, and maybe doing it in an inner constructor is also more future-proof.

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

No branches or pull requests

2 participants