Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: panflute.elements.builtin2meta no longer supports subclasses of builtins #166

Closed
dhimmel opened this issue Nov 11, 2020 · 6 comments · Fixed by #168
Closed
Assignees
Labels

Comments

@dhimmel
Copy link
Contributor

dhimmel commented Nov 11, 2020

a72ffb5 was first released in 2.0.3. It breaks support for converting subclasses of builtin types to pandoc meta types. This is because it stops using isinstance and directly compares type instead. CC @ickc

This caused the issue mentioned at manubot/rootstock#386 (comment) and #164 (comment):

  File "/usr/share/miniconda3/envs/manubot/lib/python3.7/site-packages/panflute/containers.py", line 76, in insert
    v = check_type(v, self.oktypes)
  File "/usr/share/miniconda3/envs/manubot/lib/python3.7/site-packages/panflute/utils.py", line 71, in check_type
    raise TypeError(msg)
TypeError: 

Element "MetaList" received "CSL_Item" but expected <class 'panflute.base.MetaValue'>

CSL_Item is a dict subclass.

@ickc
Copy link
Collaborator

ickc commented Nov 11, 2020

Thanks. Can you give me a MWE here?

@dhimmel
Copy link
Contributor Author

dhimmel commented Nov 11, 2020

Can you give me a MWE here?

class My_Dict(dict):
    pass

my_dict = My_Dict()
data = [my_dict]
pf.elements.builtin2meta(data)

Raises

TypeError: 
Element "MetaList" received "My_Dict" but expected <class 'panflute.base.MetaValue'>

@ickc
Copy link
Collaborator

ickc commented Nov 11, 2020

Thanks. I'm looking at it right now. In real cases would the only subclasses used be of Block and Inline? Or any of the types?

If it is only Block and Inline, then we can special case them. Else, just revert that commit.

@dhimmel
Copy link
Contributor Author

dhimmel commented Nov 11, 2020

What about my suggestion at #146 (comment)

Perhaps if this speed increase is substantial, you could try your type lookup first, and if no match fallback to isinstance.

I will submit a PR

In real cases would the only subclasses used be of Block and Inline?

I think any builtin types might be subclassed

@sergiocorreia
Copy link
Owner

Ok, I finally understand the issue a bit better. Was initially confused about it being related to _res_func but it's just about builtin2meta

@ickc
Copy link
Collaborator

ickc commented Nov 12, 2020

Right, see comment at #167 (comment)

sergiocorreia added a commit that referenced this issue Nov 12, 2020
- Fix #166
- Improve testing ( #170, #169, #164)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants