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

Stricter condition for type assignability #2902

Merged
merged 1 commit into from
May 22, 2024

Conversation

RunDevelopment
Copy link
Member

This adds a new assign function that determines whether a type T is assignable to an input with some definition type D.

The old condition was "T and D overlap." This condition served as well to prevent connects that could not be valid, but it's not enough anymore. The new Conditional node causes problems in that regard, because it allows users to create the union of 2 types A and B. So users can e.g. create the type Image | number which is assignable to a number input under the old condition.

To fix this issue, I modified the condition slightly. Instead of checking with T for overlap, it will now split T into its unioned types and check with those items. So the condition is now: split(T).every(item => overlap(item, D)). The key here is how T is split. It's a little complex, because the any type should still be assignable to everything. split essentially works like this:

function split(T) {
  if (T.isAny) return [T]
  const num = T & number
  const str = T & string
  const structs = T.iterateStructInstances()
  return [num, str, ...structs]
}

2 things to note:

  1. The is-any check is a little fuzzy to allow types very close to any. This is necessary, because nodes can't actually output any. There is special handling for the Error type, so nodes will output at most any \ Error.
  2. Iterating struct instances means grouping them by their definition. E.g. Image { width: 100 } | Image { width: 200 } | PyTorchModel will be iterated as [ Image { width: 100 } | Image { width: 200 }, PyTorchModel ].

The new assignability condition fixes the issues the Conditional node causes, but I expect that we will likely need to revise it further in the future.

Examples

Image or color:
image

Image or number:
image

@joeyballentine joeyballentine merged commit 9f3e2b2 into chaiNNer-org:main May 22, 2024
4 checks passed
@RunDevelopment RunDevelopment deleted the type-assign branch May 22, 2024 14:21
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