Numpy NDArray breaks inspection module

[mansenfranzen]

Numpy NDArray breaks inspection module

[mansenfranzen]

One more note which is interesting. This odd behaviour appears specifically with pydantic:

from pydantic import BaseModel
from inspect import isclass
from numpy.typing import NDArray

isinstance(NDArray, type) # True
isclass(NDArray) # True

issubclass(NDArray, int) # False - works
issubclass(int, BaseModel) # False - works

issubclass(NDArray, BaseModel) # Raises TypeError - doesn't work

Hence, this is some weird interaction between NDArray and pydantic's BaseModel because they work as expected with other types.

[j-carson]

I created a repo with a minimum test case https://github.com/j-carson/bug_report

[mansenfranzen]

@j-carson Thanks for reporting this!

It is interesting how numpy's NDArray behaves in conjunction with issubclass.

As the resulting error suggests (TypeError: issubclass() arg 1 must be a class), NDArray does not seem to be a class. However, exactly this is checked before via isinstance(obj, type). According to ìnspect.issclass(NDArray) it is a class, too.

Do you want to provide a PR? I think it is a good issue to start with. If not, I can do it.

[j-carson]

Are you OK with the above solution of wrapping this bit of code in a try/except block? If so, I can do a PR.

[mansenfranzen]

It is a perfect starting point - give it a go!

The more tricky part is about testing that the issue is fixed without adding numpy as a dev/test dependency. I don't want to add numpy as a dependency just because of this odd ndarray behavior. Instead, it's probably best to mimic numpy's ndarray. However, this may be difficult to achieve. An object being a class and not working with issubclass at the same time is very strange.

Interestingly, numpy itself has custom function to test for subclasses (see here) which adds a try/except block around pythons builtin issubclass.

[j-carson]

Here's another oddball - it works if I test against BaseModel's parent class, which is 'pydantic.main.ModelMetaclass'

issubclass(NDArray, BaseModel.__class__) # False, no exception raised

[j-carson]

This also looks relevant:

https://bugs.python.org/issue45326

I can use

issubclass(dict[str,str], BaseModel)

as a test case that does not require importing numpy.

Edit to add: Hmmm, I have a case where issubclass of BaseModel throws the desired exception and does not require numpy, but I'm not sure how to get autodoc to call autodoc_pydantic with this test case...

[j-carson]

OK, so the more I look at it, it's an interaction between a generic type class (Numpy implements it's own backported version so NDArray is a generic across all versions of python https://github.com/numpy/numpy/blob/main/numpy/typing/_generic_alias.py) and an abc-flavored class (BaseModel is an abstract base class).

Rather than writing a bunch of code that checks for generics based on different versions of python, the only practical way to solve this is with the try/except code above - autodoc_pydantic shouldn't be in the business of digging into the internals of types and annotations. There are a lot of libraries forced to do this (the pydantic code dealing with types is scary-complicated) and it would be a real pain to maintain it.

So, I'm happy with my solution. I'm still working through the testing.

[mansenfranzen]

This also looks relevant:
https://bugs.python.org/issue45326
I can use issubclass(dict[str,str], BaseModel) as a test case that does not require importing numpy.

That's exactly what I was thinking of. I found several other related python issues regarding issubclass bugs but I did not come across this one.

Edit to add: Hmmm, I have a case where issubclass of BaseModel throws the desired exception and does not require numpy, but I'm not sure how to get autodoc to call autodoc_pydantic with this test case...

I'm not sure if I understand you correctly but I assume you want to call autodoc_pydantic's auto-documenters to confirm that they don't fail anymore when encountering NDArray-like objects. Calling auto-documenters in the test suite has some complexity (test sphinx app required etc...). Perhaps this helps a little. But anyway I can assist you here once you've added your PR.

There is a second test option which does not require calling auto-documenters and which is even more straightforward. You can explicitly write tests for ModelInspector.static.is_pydantic_model passing all possible objects (e.g. subclasses of BaseModel or subclasses that mimic NDArray). I think you can start with this first and I can help with the auto-documenter integration test.

Rather than writing a bunch of code that checks for generics based on different versions of python, the only practical way to solve this is with the try/except code above - autodoc_pydantic shouldn't be in the business of digging into the internals of types and annotations. There are a lot of libraries forced to do this (the pydantic code dealing with types is scary-complicated) and it would be a real pain to maintain it.

I couldn't agree more 😃.

[j-carson]

I am currently just trying to get sphinx to crash again with a test case in my development environment, then I'll worry about hooking it up to pytest. I'm not having any luck reproducing it without numpy...

My little dict[str,str] test case above throws a similar exception to my problem on the oneliner subclass check, but when I try putting a dict[str,str] in my test case, somehow autodoc doesn't do the same thing anymore... Not sure why...

[mansenfranzen]

What python version do you use? Given the bug report you've mentioned earlier, it only occurs since python 3.9 upwards.

[mansenfranzen]

Also, can you share the example code you've used so far to recreate the issue?

[j-carson]

The example code that crashes is in this repo - https://github.com/j-carson/bug_report

I caught it on Python 3.7, but also see it on 3.9. (Numpy has it's own backport of generic typing.)

[j-carson]

The code that I was trying to make dict[str,str] crash was to replace the NDArray with the "Trouble" class below, but without success:

try:
    BaseClass = dict[str,str]
except TypeError:
    from typing import Dict
    BaseClass = Dict[str,str]

class Trouble(BaseClass):
    pass

While trying to figure out why autodoc was able to handle the above, I was digging into autodoc a little more deeply. I figured out I could configure

autodoc_mock_imports = ['numpy.typing']  

and my problems disappear, so there is a workaround for this issue at least.

[mansenfranzen]

For pragmatic reasons and due to the minor benefit that a custom test case mimicing numpy's NDArray really adds, I'm considering to just ignore this special case in the test suite. In fact, this test case may introduce more complexity and overhead than value it provides.

Also, it might be useful to simply use a try/except block around the issubclass because that's really the only relevant information required. If issubclass fails for whatever reason then it is not a pydantic model. Hence, there is also no need to handle any specific exceptions:

@staticmethod                                                                                   
def is_pydantic_model(obj: Any) -> bool:                                                        
    """Determine if object is a valid pydantic model.                                           
                                                                                                
    """                                                                                         
    try:                                                                                                                                                  
        return issubclass(obj, BaseModel)                                                   
    except:                                                                                     
        return False

The isinstance(obj, type) is actually not required anymore. It was used before to prevent exceptions to occur in issubclass but this is now handled via the try/except block. This latter approach that you've suggested is more robust because it handles all kinds of objects (like NDArray) which might throw some weird exceptions in issubclass.

To sum up, you're good to go with your PR without mimicing numpy's NDArray. It's enought to add test cases that succeed and fail on is_pydantic_model. What's your take on this?

[j-carson]

This is both easier to do and fine... doing this seems to be unlikely to introduce a new bug. I'll probably have time to start it on Friday and hopefully do a pull request over the weekend or early next week.

[mansenfranzen]

Sounds great, thanks! No need to hurry - take your time. I'm happy to review your PR at any point in time.