type errors on index.add(x): Arguments missing for parameter "x"

[theahura]

Summary

Note: cross posted from the pyright repository, here: https://github.com/microsoft/pyright/issues/5219

Pyright is a type checker for python. When following the faiss tutorial, pyright fails with the following type error:

Argument missing for parameter "x"

See repro below for a more concise explanation for where the error occurs.

I'm pretty sure under the hood Pyright is using the C function signature instead of the python one. Digging into the faiss python bindings, it looks like index manipulation functions like add and search are dynamically added to the index class (e.g. https://github.com/facebookresearch/faiss/blob/main/faiss/python/class_wrappers.py#L214). They are also untyped. I'm not super familiar with how faiss actually builds -- I was expecting to see a base IndexFlat class somewhere so I could peek at the types on that file, but it looks like it gets generated by swig? Either way, though I'm currently using pyright I'm not sure if any python type checkers can successfully type this library.

Curious if this is a known issue, and if there are any additional resources I can look into here

Platform

NA

Reproduction instructions

install pyright:

pip install pyright

add the following code to foo.py

import faiss
import numpy as np
emb = np.random.rand(800, 5)
idx = faiss.IndexFlatIP(emb.shape[-1])
idx.add(emb)  # works at runtime, but pyright fails with error: "Arguments missing for parameter 'x'"
idex.add(len(emb), emb)  # pyright is happy, but this fails at runtime because the wrong number of args are given

run pyright:

pyright foo.py

[theahura]

See the following comment from the maintainers of Pyright: https://github.com/microsoft/pyright/issues/5219#issuecomment-1572653343

Copying the relevant section below:

It appears that the library is dynamically monkey patching the add method with a replacement called replacement_add, and the replacement method has a different signature than the original. I don't know the reason behind this, but this won't work with static type checkers or language servers.

[mdouze]

Faiss is not quite typecheck ready. What would be needed to add it? From your comments it seems that adding proper type annotations in replacement_X would not be enough right?

[mdouze]

The current implementation lets SWIG generate naive wrappers, then the replacement_X functions translate the numpy parameters into SWIG-visible pointers.

It should be translate the replacement_X functions from python to SWIG using typemaps. However, it could be quite a lot of work. First step would be to do it for a single function.

[theahura]

From your comments it seems that adding proper type annotations in replacement_X would not be enough right?

I'm not an expert, but based on my understanding I do not think adding the type annotations in replacement_x functions would be sufficient (though it may be a good start!)

One approach may be to have a base class in Python that has the right functions and the right signatures, and then have the swig-generated class extend from that?

Tbh it's hard to suggest alternatives without fully understanding the reasoning behind why FAISS monkeypatches functions to begin with :thinking:

[g-i-o-r-g-i-o]

same "wrong" notification on google colab

Argument missing for parameter "x"(reportGeneralTypeIssues) def replacement_add(x)

Open in tab View source

Adds vectors to the index.