8000 Bug fix binary expression in updates to grammar by kl2806 · Pull Request #1869 · allenai/allennlp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

Bug fix binary expression in updates to grammar #1869

Merged
merged 4 commits into from
Oct 5, 2018
Merged

Bug fix binary expression in updates to grammar #1869

merged 4 commits into from
Oct 5, 2018

Conversation

kl2806
Copy link
Contributor
@kl2806 kl2806 commented Oct 5, 2018 •
edited
Loading

Fixes a bug where the base grammar binary expressions get mutated due to shallow copy. Grammar objects contain expressions i.e. OneOf that have tuples of the members that we want to modify. Just modifying the members will mutate the old copy.

https://github.com/erikrose/parsimonious/blob/master/parsimonious/grammar.py#L82 The docs in parsimonious states that all expressions are created from scratch but shallow copy doesn't actually clone child objects. Below is a simple example of the issue:

>>> from parsimonious import Grammar
>>> my_grammar = Grammar(r"""
...     styled_text = bold_text / italic_text
...     bold_text   = "((" text "))"
...     italic_text = "''" text "''"
...     text        = ~"[A-Z 0-9]*"i
...     """)
>>> new_grammar = my_grammar._copy()
>>> new_grammar['styled_text'].members
(<Sequence bold_text = "((" text "))">, <Sequence italic_text = "''" text "''">)
>>> new_grammar['styled_text'].members = new_grammar['styled_text'].members + (new_grammar['italic_text'],)
>>> new_grammar['styled_text'].members
(<Sequence bold_text = "((" text "))">, <Sequence italic_text = "''" text "''">, <Sequence italic_text = "''" text "''">)
>>> my_grammar['styled_text'].members
(<Sequence bold_text = "((" text "))">, <Sequence italic_text = "''" text "''">, <Sequence italic_text = "''" text "''">)

@matt-gardner matt-gardner merged commit 3989000 into allenai:master Oct 5, 2018
@kl2806 kl2806 deleted the fix-grammar branch October 16, 2018 17:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matt-gardner matt-gardner matt-gardner approved these changes

@DeNeutoy DeNeutoy Awaiting requested review from DeNeutoy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kl2806 @matt-gardner
0