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

Should == over ComposedFunction support missing as usual #54881

Open
nsajko opened this issue Jun 21, 2024 · 6 comments
Open

Should == over ComposedFunction support missing as usual #54881

nsajko opened this issue Jun 21, 2024 · 6 comments
Labels
equality Issues relating to equality relations: ==, ===, isequal needs decision A decision on this change is needed

Comments

@nsajko
Copy link
Contributor

nsajko commented Jun 21, 2024

The current behavior is:

julia> (sin  missing) == (sin  missing)
true

But it seems like we should actually have == return missing in this case? This is because missing basically represents a wildcard function here, the way I interpret it at least. This would be analogous to, e.g.:

julia> (missing, 3) == (missing, 3)
missing

This is a special case to consider for #53853.

Not sure if I should label this for triage, but it'd be nice to get a quick decision here, given that it's relevant to @jw3126's PR #54877.

@nsajko nsajko added needs decision A decision on this change is needed equality Issues relating to equality relations: ==, ===, isequal labels Jun 21, 2024
@adienes
Copy link
Contributor

adienes commented Jun 21, 2024

please, no

I think Missing should stay in its statistics corner and only propagate in statistical contexts

Imo it already propagates through far too many functions.

@nsajko
Copy link
Contributor Author

nsajko commented Jun 21, 2024

For better or worse, the ternary logic including missing is part of the semantics of ==.

@adienes
Copy link
Contributor

adienes commented Jun 21, 2024

wait, why does missing work with in the first place? it's not a function

I'd sooner special case an error path for ∘(::Missing) than make it propagate.

@jariji
Copy link
Contributor

jariji commented Jun 22, 2024

But it seems like we should actually have == return missing in this case?

If ==((;f1,g1)::ComposedFunction, (;f2,g2)::ComposedFunction) = f1 == f2 && f2 == g2, then this should return missing. I don't think there's another result that makes sense.

why does missing work with in the first place? it's not a function

takes any value, it doesn't know if they're callable until it tries to call them. That's basically the julia style. If it's necessary to know statically that there's a problem then JET will detect the problem at the problem where the invalid call occurs, not where the construction occurs. Just like you can make a list of [abs, exp, missing] and it doesn't error until you try to call them.

@jw3126
Copy link
Contributor

jw3126 commented Jun 22, 2024

If ==((;f1,g1)::ComposedFunction, (;f2,g2)::ComposedFunction) = f1 == f2 && f2 == g2, then this should return missing. I don't think there's another result that makes sense.

I don't think so, && will error on missing first argument:

julia> missing && true
ERROR: TypeError: non-boolean (Missing) used in boolean context
Stacktrace:

@jariji
Copy link
Contributor

jariji commented Jun 22, 2024

Ah yeah, true && missing returns missing and missing && true errors. So if the implementation is structural, the result might depend on the order in which the components are checked. ComposedFunction components have an obvious "order" to them (namely, application order), so it seems fine to implement == structurally using the application order.

I don't like the idea of introducing a special case error specifically to make sure missing doesn't propagate: Missing is the type designed for propagating missingness, after all. 🙂

If a user wants to create a collection of callables, some of which some are missing, and then them together, I think they should be able to do that, and expect the behavior to compose too, following the specifications of the components (==, ComposedFunction, and Missing), without needing to also consider a special rule that only takes effect when these three things are combined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
equality Issues relating to equality relations: ==, ===, isequal needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

4 participants