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

antlr runtime version #344

Open
fbergmann opened this issue Dec 18, 2024 · 4 comments
Open

antlr runtime version #344

fbergmann opened this issue Dec 18, 2024 · 4 comments

Comments

@fbergmann
Copy link
Collaborator

Currently the requriements for petab are antlr4-python3-runtime==4.13.1 is petab really incompatible with 4.13.2?

@dweindl
Copy link
Member

dweindl commented Dec 18, 2024

From Antlr:

It is vital that the versions for the
Antlr tool used to generate the parser
and the Antlr Python3 runtime match.
E.g., 4.13.0. Using build files will help
eliminate common errors from happening.

I guess, it may or may not work, but if it doesn't work it will be a pain to debug.

Does the requirement interfere with any other package that might be used in combination?

The only way I currently see to resolve this would be generating the parser during package installation instead of shipping the generated parser code.

@fbergmann
Copy link
Collaborator Author

It was some experiments using localstack that required 4.13.2. Maybe we could get around it restricting it to be >= 4.13.1 and < 4.14? I modified my local installation for now. And at least what i needed of libpetab is working fine with that

@dweindl
Copy link
Member

dweindl commented Dec 18, 2024

For their Python package, there is a runtime check that ignores the patch number, so I guess this might be fine: https://github.com/antlr/antlr4/blob/b91cecf6d06600433a12a12271a7985d2845d7aa/runtime/Python3/src/antlr4/Recognizer.py#L36-L41

However for JS, they also require the same patch number...
https://github.com/antlr/antlr4/blob/b91cecf6d06600433a12a12271a7985d2845d7aa/runtime/JavaScript/src/antlr4/Recognizer.js#L17-L22

I am open to loosening the requirement to some minor version for now, but I guess, that will just postpone the problem...

The only way I currently see to resolve this would be generating the parser during package installation instead of shipping the generated parser code.

This seems to be the proper fix, but it would mean additionally requiring antlr4-tools, which seems to be less smooth on Windows.

@fbergmann
Copy link
Collaborator Author

The windows story sounds ghastly indeed, as someone that happens to use the platform regularly, i can tell you that just as in any platform if one just grabs miniconda, or creates virtual environments things are much easier.

That being said, i'm not advocating of changing the approach. I'd rather not need a java runtime just to install a python package. so lets not do that. Better to add a section to the libpetab-python readthedocs page on how i'd go ahead and regenerate the parser if i needed it and add a link to that behind the ==4.13.1. Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants