isinstance(attr, Attr)
Opened this issue · 4 comments
will fail even when attr
is a correctly formed tuple. Attr
is useful as a piece of documentation since in the REPL one can do for example
>>> Image
Image(Attr, [Inline], Target)
>>> Attr
Attr = (Text, [Text], [(Text, Text)])
but the attributes produces by a pandoc read is a mere tuple, not an Attr
instance. Actually Attr
cannot be instantiated at all. So while the documentation aspect is handy (to avoid the inlining of Attr
definition everywhere it is used), the existence of the class may also be misleading since it cannot be used in pattern matching.
Consider using some custom __instancecheck__
class method (at the metaclass level) for Typedef
instances to have the proper behavior [*] ? We could check structural conformity of the "instance", say check that we have a tuple of length 2 with two Text
components and thus consider that it's a valid Target
. But pattern-matching the whole document for such pattern could still lead (theoretically) to false positives (tuples that just happen to have a structure consistent with Target
). A real example would be : we have a key-value component of an Attr
; this is not a target, but would pattern-match. So such pattern-matching would be unsafe ...
So the "proper" way to deal with this would be not to match for Typedefs, but match the parents in the type hierarchy instead. That's easy for Target
(only two parents : Image
and Link
). This is less acceptable for Attr
: they are used by a great number of types and the Attr
argument is not even always in first position (for Header
, this is the second argument ...). So a general "find all Attrs and do something with them" would require some significant amount of boilerplate.
[*} See https://docs.python.org/3/reference/datamodel.html#customizing-instance-and-subclass-checks
So such pattern-matching would be unsafe ...
We could consider making an instance check on a Typedef
throw an exception, instead of allowing a construct which won't do what the user is expecting, for non-obvious reasons.
Well actually there are two sides to this:
-
pattern matching on typedefs is a lousy, unsafe practice, so that pleads for an error on isinstance,
-
but an isinstance that implements some structural typechecking would be handy for typechecking.
Since we are all consenting adults ... structural typecheking would be the way to go. Plus a decent amount of documentation to teach that it should be avoided in pattern matching. There is already a footnote on this subject in the cookbook.
The underyling logic should already be present in the typecheck
branch actually.