For annotating classes and methods in swh/indexer/indexer.py, swh/indexer/metadata.py, swh/indexer/mimetype.py, swh/indexer/origin_head.py and swh/indexer/rehash.py.
- Group Reviewers
- Maniphest Tasks
- T2258: Add type annotations to indexer classes
- rDCIDX5f49b59e6aa3: Add type annotations to indexer classes
- Required Signatures
L3 Software Heritage Contributor License Agreement, version 1.0
In swh-indexer's indexer.py (code), the run() method of BaseIndexer class has been overriden by a few derived classes. However, I think some of these overridings may be flawed, since when I try to type-annotate them, they're giving the mypy errors: argument of method incompatible with supertype / Signature of "run" incompatible with supertype "BaseIndexer". This is probably because of a violation of the Liskov substitution principle.
Do you'll suggest that I just tell mypy to ignore these errors for now? I can't get mypy to ignore these errors in the method definition expression either. The expression is spread across multiple lines, and the ignore annotation works for these kinds of expressions only in Python 3.8+ (source). So, I could just remove type checking from these methods altogether, for now.
These are the errors I'm getting:
Add type annotations for rehash.py, indexer.py, origin_head.py and metadata.py.
Overriden methods and their parent class methods have mostly been excluded from type annotating, for now.
A lot of the methods in origin_head.py require documentation and are incomplete/unused, so I've had to use a lot of Any's in typing those.
I've set the type annotation as Any if
- I'm unsure of the type
- The field has values of different types, like in the value field of some Dict's or in some elements of some Tuple's
- The type of the variable changes in the program.
Thanks, looks good.
Remains some remarks.
And another one more general, for the docstring, either wrap all parameters with **parameter-name**: description or remove them altogether.
I see you kinda mix those.
Add the working directory docstring, it's where the file is written.
yeah, it's fine for now 
The one that matters here is the main class BaseIndex's index definition which you typed.
 indexer has grown a bit and after a while and getting back to it, it's a tad unclear.
remove the **
str without the quote :)
Done. I've left the single ** for the kwargs in the docstring though.
Should I do this?
Added to the docstring.
Regarding the type of working_directory, I wasn't able to figure it out based on checking its use in this method, as well as from checking the methods that call this method through using grep. According to this, the type can either be either a string or bytes object representing a path. . So if we're sure it's a string, I'll change it to str 👍
Ah I see
SendType should be None, not Any. https://docs.python.org/3/library/typing.html#typing.Generator
same comment on SentType
sorry I missed this earlier, but you also forgot yield type for this generator