sergiocorreia / panflute

An Pythonic alternative to John MacFarlane's pandocfilters, with extra helper functions
http://scorreia.com/software/panflute/
BSD 3-Clause "New" or "Revised" License
500 stars 59 forks source link

close #166: Revert "repeat for builtin2meta" #167

Closed ickc closed 4 years ago

ickc commented 4 years ago

This reverts commit a72ffb5b48ca2c871ad8818595cf0ae888c91f4c.

sergiocorreia commented 4 years ago

I wouldn't go as far as revert it yet, want to understand if there is a middle point that preserves the speed and the flexibility

ickc commented 4 years ago

c.f. #166

Sorry didn't foresee people subclassing those. After the discussion in #166, I think we should keep it general.

Since the comments indicates that only from_json is slow, but didn't mention builtin2meta, the trick there is nice to have but non-essential. isinstance is much more general (but slow).

ickc commented 4 years ago

For such a short if then else clause, the speed up wouldn't be much. The real slow down is from isinstance comparing to just look up type, but as long as we allow people to subclass things then there really isn't a get away.

Look up first then repeat if isinstance then else clause is going to slow down the last case.

dhimmel commented 4 years ago

As noted in https://github.com/sergiocorreia/panflute/issues/166#issuecomment-725720122, I am working on a fix. Would appreciate the chance to propose my PR.

ickc commented 4 years ago

Feel free and your call.

sergiocorreia commented 4 years ago

Thanks again for both of your comments.

So far it seems there are two options:

1) keep the if tag in _res_func: part, and at the end instead of raising NotImplementedError, just repeat the preexisting code. The con of this is that it would add a lot of code and when we add new elements (like Underscore) we would need to remember to add them in both places.

2) use the bases function:


from panflute import *

class Foo:
    pass

class Bar(Foo):
    pass

class SuperSpace(Space, Bar):
    pass

x =SuperSpace('hello')

print(type(x))
print(type(x).__bases__)