unistra / python-glpi-api

Python module for interacting with GLPI using the API.
GNU General Public License v3.0
18 stars 10 forks source link

Type stubs? #9

Open lightspot21 opened 2 years ago

lightspot21 commented 2 years ago

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!

fmenabe commented 2 years ago

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!

lightspot21 commented 2 years ago

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!