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

Bump lark to v1 #1230

Closed
merkys opened this issue Jun 8, 2022 · 5 comments · Fixed by #1231 or #1242
Closed

Bump lark to v1 #1230

merkys opened this issue Jun 8, 2022 · 5 comments · Fixed by #1231 or #1242
Assignees
Labels
dependency_updates Issues pertaining to updates to our dependencies that are breaking the eager build

Comments

@merkys
Copy link
Member

merkys commented Jun 8, 2022

The code currently depends on lark-parser v0.12.0. Since then, a stable release of lark, v1, has been released. Are there any plans to update? There are some API changes, but I like the prospect of depending on API with some promises for backwards compatibility.

@ml-evs
Copy link
Member

ml-evs commented Jun 8, 2022

Thanks for pointing this out @merkys, I see we are currently depending on lark-parser from PyPI, rather than lark. I am not sure when they changed names, but I feel they should have released a patch for v0.12 of lark-parser that indicated the package was being renamed.... I will make a PR that first switches over to the correct named package, then bumps the version.

@ml-evs ml-evs added the dependency_updates Issues pertaining to updates to our dependencies that are breaking the eager build label Jun 8, 2022
@ml-evs ml-evs self-assigned this Jun 8, 2022
@merkys
Copy link
Member Author

merkys commented Jun 9, 2022

Thanks for prompt response, @ml-evs. I have seen the same build/test failures as in #1231 when I was trying to drop-in replace lark on my machine.

@ml-evs
Copy link
Member

ml-evs commented Jun 9, 2022

Thanks for prompt response, @ml-evs. I have seen the same build/test failures as in #1231 when I was trying to drop-in replace lark on my machine.

Yes, it seems like they made lark.visitors.Transformer itself an ABC which messes up the inheritance. If I get around this (using e.g., metaclass=abc.ABCMeta and other tricks) then unfortunately parsing still does not work (and just returns null without any errors). I may ask on the lark forums about this.

@ml-evs ml-evs reopened this Jun 9, 2022
@ml-evs
Copy link
Member

ml-evs commented Jun 9, 2022

With some help from the Lark devs on gitter, #1242 should fix our compatibility issues with v1, which were to do with how the grammar was loaded (maybe_placeholders should be set to true, whereas v1 changed the default to false) @merkys

@merkys
Copy link
Member Author

merkys commented Jun 10, 2022

I confirm the fix indeed works. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependency_updates Issues pertaining to updates to our dependencies that are breaking the eager build
Projects
None yet
2 participants