python-glpi-api icon indicating copy to clipboard operation
python-glpi-api copied to clipboard

Type stubs?

Open lightspot21 opened this issue 4 years ago • 2 comments

Hello,

Is it possible to provide type hints for the API functions? Many people (myself included) work with static type checkers like mypy or pylance to make sure up to a point that no arbitrary conversions happen between data that haven't been accounted for.

Thanks in advance!

lightspot21 avatar Nov 23 '21 12:11 lightspot21

Sorry for the late answer, I started last week but could not continue until today.

I managed to add type hints (inside the type_hint branch for now) but still have some errors. I bypass them by adding # type: ignore but it feels like cheating.

The first error is due to some hack to support both Python 2 and 3:

#def _raise(msg: Union[str, bytes])
def _raise(msg: str) -> None:
    if sys.version_info.major < 3:                                                                     
        msg = msg.encode('utf-8') # type: ignore                                                       
    raise GLPIError(msg)

It raises the error glpi_api.py:46: error: Item "bytes" of "Union[str, bytes]" has no attribute "encode". I don't see how to solve it because between str and bytes from Python 2 and Python 3, there is always one that will be invalid. Except for the ignore hack, dropping Python 2 support seems the only other solution (but it should probably been dropped anyway!).

The second error happen when session headers are set:

        # Set required headers.                                                                        
        self.session.headers = {                                                                       
            'Content-Type': 'application/json',                                                        
            'Session-Token': session_token,                                                            
            'App-Token': apptoken                                                                      
        } # type: ignore

mypy raise glpi_api.py:121: error: Incompatible types in assignment (expression has type "Dict[str, Any]", variable has type "CaseInsensitiveDict[str]"). Tried a few thing but nothing worked!

It is the first time I add type hints to a library so if you have ideas to better solve theses errors or have feedback to give, it is welcomed!

fmenabe avatar Nov 30 '21 16:11 fmenabe

Oh wow I'm late as heck, just saw the message :|

Regarding the _raise hack, maybe add the encode behind an isinstance() check? Mypy should be clever enough to infer the types correctly. If that doesn't work, maybe this answer from SO could help? https://stackoverflow.com/questions/57804661/how-to-help-mypy-understand-if-isinstance-else-structure

As for the last, CaseInsensitiveDict looks like an internal requests data structure. However it provides dict's update() method (https://www.kite.com/python/docs/requests.structures.CaseInsensitiveDict). So, instead of directly assigning, why not just call update like this?

self.session.headers.update({
     'Content-Type': 'application/json',                                                        
     'Session-Token': session_token,                                                            
     'App-Token': apptoken
})

Hope this helps!

lightspot21 avatar Dec 17 '21 12:12 lightspot21