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

Relax dependency on antlr-python3-runtime #2699

Closed
jlopezpena opened this issue Jul 4, 2023 · 14 comments · Fixed by #2733
Closed

Relax dependency on antlr-python3-runtime #2699

jlopezpena opened this issue Jul 4, 2023 · 14 comments · Fixed by #2733
Labels
dependencies Pull requests that update a dependency file enhancement Enhanvement request

Comments

@jlopezpena
Copy link
Contributor

jlopezpena commented Jul 4, 2023

🚀 Feature Request

Allow installing hydra with a more recent version of antlr

Motivation

I am trying to upgrade my repos to use pandas v2, and in one of them I got blocked by hydra. Specifically, datacompy version 0.10.2 (min required version for pandas 2) requires fugue which requires antlr runtime >= 4.11, but hydra pins the antlr version 4.9, which makes it impossible to resolve the dependencies.

Pitch

Provided that it doesn't break anything in hydra, a simple relaxation of the dependency on antlr-python3-runtime so that it allows versions greater than 4.11 would solve the dependency lock. Version 4.9 of antlr is over two years old.

Are you willing to open a pull request?
Happy to, it should be a very easy one, provided that there are no API breaking changes in antlr!

Additional context

Here is the relevant part poetry dependency resolution error, for completeness:

Because no versions of datacompy match >0.10.2,<0.11.0
 and datacompy (0.10.2) depends on fugue (0.8.5), datacompy (>=0.10.2,<0.11.0) requires fugue (0.8.5).
    And because fugue (0.8.5) depends on fugue-sql-antlr (>=0.1.6), datacompy (>=0.10.2,<0.11.0) requires fugue-sql-antlr (>=0.1.6).
(1) So, because no versions of fugue-sql-antlr match >0.1.6
 and fugue-sql-antlr (0.1.6) depends on antlr4-python3-runtime (>=4.11.1,<4.12), datacompy (>=0.10.2,<0.11.0) requires antlr4-python3-runtime (>=4.11.1,<4.12).

    Because no versions of hydra-core match >1.3.0,<1.3.1 || >1.3.1,<1.3.2 || >1.3.2,<2.0.0
 and hydra-core (1.3.0) depends on antlr4-python3-runtime (==4.9.*), hydra-core (>=1.3.0,<1.3.1 || >1.3.1,<1.3.2 || >1.3.2,<2.0.0) requires antlr4-python3-runtime (==4.9.*).
    And because hydra-core (1.3.1) depends on antlr4-python3-runtime (==4.9.*)
 and hydra-core (1.3.2) depends on antlr4-python3-runtime (==4.9.*), hydra-core (>=1.3.0,<2.0.0) requires antlr4-python3-runtime (==4.9.*).
    And because datacompy (>=0.10.2,<0.11.0) requires antlr4-python3-runtime (>=4.11.1,<4.12) (1), datacompy (>=0.10.2,<0.11.0) is incompatible with hydra-core (>=1.3.0,<2.0.0)
@jlopezpena jlopezpena added the enhancement Enhanvement request label Jul 4, 2023
@shagunsodhani
Copy link
Contributor

Hey @jlopezpena ! Thank you for brining this up. A PR is very much appreciated. I dont think updating antlr should break anything. cc @Jasha10 @odelalleau

@shagunsodhani shagunsodhani added the dependencies Pull requests that update a dependency file label Jul 14, 2023
@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 14, 2023

antlr4-python3-runtime, which is a python package, is supposed to be the same minor version as the version of antlr4 that is used to generate the contents of the hydra/grammar/gen folder (see omry/omegaconf#798).

Currently Hydra is using antlr4 version 4.9.3 (see the jar file stored in the hydra/build_helpers/bin folder of this repository).

If we upgrade antlr4-python3-runtime to a different minor version, we'd also need to replace that antlr jar file and any references to it in the codebase.

@shagunsodhani
Copy link
Contributor

Thanks for the clarification @Jasha10. Do you see any downside to this ? I am not at all familiar with this piece of code.

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 14, 2023

I heard that Meta uses a specific antlr4 version in it's monorepo. I'm not sure if changing the version here would cause issues for internal usage.

Besides such a compatibility concern, I don't see any issue with upgrading.
We'd probably want to upgrade the version of antlr4 used by OmegaConf at the same time. (cc @omry).

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 14, 2023

Here's a brief background on the role of antlr4 in Hydra/OmegaConf:

OmegaConf/Hydra use antlr4 to compile several .g4 grammar files.

When you build the omegaconf or hydra-core python package, the antlr4 .jar file is used to compile these .g4 files into some .py files that are stored in the omegaconf/grammar/gen folder or the hydra/grammar/gen folder. Setuptools drives this process: the setup.py file loads build_helpers/build_helpers.py, which in turn invokes the antlr4 .jar file.

At runtime, when OmegaConf / Hydra want to parse a cli override or variable interpolation, they load the .py files from the grammar/gen folder to do the parsing. These .py files depend on the antlr4-python3-runtime python package.

@odelalleau
Copy link
Collaborator

I heard that Meta uses a specific antlr4 version in it's monorepo. I'm not sure if changing the version here would cause issues for internal usage.

Are you talking about the version of the antlr binary, or the antlr4-python3-runtime package? (we only care about the latter for users)

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 14, 2023

I'm not 100% sure.

My assumption is that, within Meta, buck calls into setuptools to build the hydra-core package. Since Hydra's setup.py file calls the antlr4 .jar file that's vendored in this repository, I think that the version of the antlr4 binary that's used broadly in Meta's monorepo wouldn't matter too much. Rather, what would matter is that Meta's vendored version of antlr4-python3-runtime matches the version of the .jar file that's vendored in this repo.

I'm not an expert in Meta's monorepo though and I could be making some bad assumptions here.

@jlopezpena
Copy link
Contributor Author

So, if I understood correctly, this PR would simply entail updating the jar file in hydra/build_helpers/bin by antlr 4.11 (or whatever), as well as the reference to it in the antlr4 script in the same folder, and then change the dependency on antlr4-python3-runtime to match the jar. I can make a PR for that and test that hydra test suit runs, but have no way of tracking if this could cause issues with Meta's internal repos

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 18, 2023

@jlopezpena that's basically correct.

Since omegaconf also relies on antlr4-python3-runtime, we have to be careful or else we'll get incompatible version requirements in omegaconf vs hydra (where each of them points to a different version of the antlr python runtime). To avoid this, we'd need to upgrade omegaconf's dep on antlr first and do an omegaconf release. We could then update Hydra to depend on the new versions of omegaconf and antlr.

Files affected in omegaconf:

  • omegaconf/build_helpers/bin/antlr-4.9.3-complete.jar
  • omegaconf/build_helpers/build_helpers.py (replace the reference to the .jar file)
  • omegaconf/requirements/base.txt:
    • Update dep on antlr4-python3-runtime

Files affected in Hydra:

  • hydra/build_helpers/bin/antlr-4.9.3-complete.jar
  • hydra/build_helpers/build_helpers.py (replace the reference to the .jar file)
  • hydra/build_helpers/bin/antlr4 (replace the reference to the .jar file)
  • hydra/build_helpers/bin/grun (replace the reference to the .jar file)
  • hydra/requirements/requirements.txt:
    • Update dep on antlr4-python3-runtime
    • Update dep on omegaconf

See OmegaConf PR omry/omegaconf#912 and Hydra PR #2192 from when we upgraded antlr v4.8 -> v4.9.

See also my comment here for a recipe to test out such changes locally or build hydra/omegaconf wheels with a custom antlr version.

@skshetry
Copy link
Contributor

skshetry commented Aug 3, 2023

anltr4-python3-runtime>=4.11.0 publishes wheels now (antlr/antlr4#3805), so it's also faster to install.

@jlopezpena
Copy link
Contributor Author

Created PRs in both omegaconf and hydra. The PR in omegaconf should be merged, and a new version released, before the PR in hydra can be merged. Let me know if there is anything else I can do!

@Jasha10 Jasha10 linked a pull request Feb 15, 2024 that will close this issue
@skylarbpayne
Copy link

Thanks for the fix! Is there a process for this to be released for hydra-core?

@Daraan
Copy link

Daraan commented Jun 24, 2024

Is there a chance to get this as a pre-release? I am also running frequently into antlr issues because of dependency issues.

Edit:
@shchur Thank your for the reply, but no I am using another project that uses antlr 4.11.

@shchur
Copy link
Contributor

shchur commented Jun 24, 2024

@Daraan in case your problem is caused by incompatibility with fugue (as was the case for me), the recently released fugue>=0.9.0 removed the antlr-python3-runtime dependency, which fixed the incompatibility with omegaconf and hydra.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement Enhanvement request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants