boisgera/pandoc

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.