mkdocstrings
AliasResolutionError on `getattr` problematic for things like deepcopy
first off: love the library. The API and source are very enjoyable. I look forward to working with it more.
Describe the bug
Naively, one would expect getattr(alias, 'nonexistent', None) to return None, but it can very often raise an AliasResolutionError (even in cases where we don't care if the alias is resolvable)
To Reproduce
from copy import deepcopy
from griffe.loader import GriffeLoader
loader = GriffeLoader()
mod = loader.load_module('griffe')
deepcopy(mod)
# or even just the dict form
deepcopy(mod.as_dict())
raises the following (starting at the point where deepcopy tries to check for a __deepcopy__ attribute:
File ~/miniconda3/envs/junk/lib/python3.10/copy.py:151, in deepcopy(x, memo, _nil)
149 y = _deepcopy_atomic(x, memo)
150 else:
--> 151 copier = getattr(x, "__deepcopy__", None)
152 if copier is not None:
153 y = copier(memo)
File ~/griffe/src/griffe/dataclasses.py:835, in Alias.__getattr__(self, name)
833 self._passed_through = True
834 try:
--> 835 attr = getattr(self.target, name)
836 except CyclicAliasError as error:
837 raise CyclicAliasError([self.target_path] + error.chain)
File ~/griffe/src/griffe/dataclasses.py:944, in Alias.target(self)
935 """Resolve and return the target, if possible.
936
937 Upon accessing this property, if the target is not already resolved,
(...)
941 The resolved target.
942 """
943 if not self.resolved:
--> 944 self.resolve_target()
945 return self._target
File ~/griffe/src/griffe/dataclasses.py:970, in Alias.resolve_target(self)
968 resolved = self.modules_collection[self.target_path]
969 except KeyError as error:
--> 970 raise AliasResolutionError(self.target_path) from error
971 if resolved is self:
972 raise CyclicAliasError([self.target_path])
AliasResolutionError: Could not resolve __future__.annotations
I tried to muck with the __getattr__ here to raise an AttributeError if self.target is unresolvable... but ran into recursion errors
Expected behavior `getattr(unresolvable_alias, 'nonexistent', None) == None
System (please complete the following information):
-
griffeversion: 4925563ef13cf8f6c466ffc3d9ec2bc373f44a09 - Python version: 3.10
- OS: macOS
Thanks :heart_eyes: I'm so glad you like it! What's your use case by the way? I think you're the first to use it directly rather than through mkdocstrings (or through the CLI) :slightly_smiling_face:
Yeah this magic __getattr__ has caused me a lot of pain as well, it's a part of the API that I don't really like, but I could not find alternatives that provided the same level of convenience. Well, that got me thinking. Currently, everything is proxied to the target (therefore, trying to resolve it), except for attributes that are redefined in the Alias class itself. Maybe we could do the opposite: forward nothing, i.e. remove __getattr__, and write proxies for each "target" attribute.
Now, as a quicker workaround: what if we add an AttributeError base to the AliasResolutionError and CyclicAliasError exceptions? Could you try that? I believe getattr would be able to catch those and return the provided default value instead.
EDIT: tested myself, same result as with your mentioned test before: recursion error :thinking:
What's your use case by the way? I think you're the first to use it directly rather than through mkdocstrings (or through the CLI) 🙂
awesome :) Yeah, I found it via mkdocstrings, but I'm generally interested in package analysis and so I started digging a bit. I'm a dev on napari/napari, but also maintain a number of ancillary packages for which I'm always trying to improve my dev process.
what I really want (as I suspect you do too 😂) is API-diffing for detecting API breakages, preferably via CI. I know this is on your future todo list, but I took a starting stab at it here: https://github.com/tlambert03/griffediff
I wouldn't actually publish that to PyPI until it's more mature. Mostly just playing around for now, and would be more than happy to port it here/collaborate on it if you are interested. (I'm sure you have some better ideas about how to go about it!)
Yeah this magic getattr has caused me a lot of pain as well, it's a part of the API that I don't really like, but I could not find alternatives that provided the same level of convenience.
I hear ya there. somewhat similarly: generally dealing with resolving forward references has been a bane of multiple projects for me too.
EDIT: tested myself, same result as with your mentioned test before: recursion error 🤔
darn :/
what I really want (as I suspect you do too joy) is API-diffing for detecting API breakages, preferably via CI. I know this is on your future todo list, but I took a starting stab at it here: https://github.com/tlambert03/griffediff
That's fantastic!! I'm definitely interested in helping you porting it to Griffe. Or not! Maybe it's better if both tools stay decoupled :slightly_smiling_face: I'd be thrilled anyway and would definitely advertise your API-diffing project in Griffe itself. The benefit I see in integrating it in Griffe though, either directly or as an extension, is that the API-diff data could then be used in upper tools like mkdocstrings to render even more useful things in HTML docs.
(I'm sure you have some better ideas about how to go about it!)
Actually I don't :laughing: I only thought about what to consider for breaking changes. I didn't think at all about data structures or any other technical details. I'll enjoy reading your code :yum:
Back to the issue: I'll investigate a bit more, and if I can't find a quick fix, I'll try the previously mentioned approach of completely removing __getattr__.
That's fantastic!! I'm definitely interested in helping you porting it to Griffe. Or not! Maybe it's better if both tools stay decoupled 🙂 I'd be thrilled anyway and would definitely advertise your API-diffing project in Griffe itself.
Awesome. Yeah, I can see benefits both ways as well. I'll open a dedicated issue here just so that anyone also interested in the topic can be aware/join the discussion. And you can post your thoughts in your own repo