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 constraints to maxime compatibility #234

Closed
baggiponte opened this issue Dec 20, 2023 · 14 comments
Closed

Relax dependency constraints to maxime compatibility #234

baggiponte opened this issue Dec 20, 2023 · 14 comments
Assignees
Milestone

Comments

@baggiponte
Copy link
Contributor

Hi! Thanks for the amazing work 😊 NeMo is basically the best OSS LLM Security framework out there.

However, I cannot use nemo at work because of some dependency conflicts. I looked at the source code and found that you pin all of your dependencies - including setuptools (do you actually use that or it's there just as a build dependency?).

Aside from some known incompatibilities, what is the reasoning behind pinning all of them? They are all fairly common and the chances of conflicts may arise: for example, pydantic is still < 2 which might clash with other LLM frameworks that use it extensively, such as llama-index or instructor.

If you'd like to consider, I'd volunteer to help with the following:

  • Narrow down the constraints
  • Possibly migrate to a modern, standard compliant project structure. Many projects nowadays go with the recommended pyproject.toml setup 1 2, which would allow you to replace a bunch of files, centralise tooling configuration (e.g. pytest).
    • More importantly, in this file you can write the [build-system] table (fear not, it's literally just two lines of toml) to list the dependencies necessary to build the package.

Thanks for your time!

@drazvan
Copy link
Collaborator

drazvan commented Dec 20, 2023

Hi @baggiponte!

Yes, we can relax the dependencies for common libraries. But for langchain for example, we should keep it pinned as we've seen multiple times that breaking changes were introduced.

We actually have a PR that started integrated Poetry (#31), but I think it's stale now. If you want to submit a PR with the changes you suggested, that would be hugely appreciated. Thanks!

@drazvan drazvan added this to the v0.7.0 milestone Dec 20, 2023
@baggiponte
Copy link
Contributor Author

Thanks for the prompt reply!

Totally agree to keep langchain pinned.

I'm not a great poetry fan. It's a great tool that brought about incredibly high devx standards for python packages, but I don't agree with some design choices. It does not comply with several PEPs (517 518 621) and defaults to upper boundary constraints [1] [2] (which might be a thing in JS but definitely not in Python).

I personally use PDM (so does pydantic). It doesn't really matter in the end: so long as the package manager complies with these PEPs, you can <package-manager> install any repo that has a [build-system].

Will get to work tomorrow!

1
2

@drazvan
Copy link
Collaborator

drazvan commented Dec 21, 2023

Let's go with PDM. Thanks!

@jordanrfrazier
Copy link

+1, with the additional ask that the langchain pin be relaxed as well, now that they've released v0.1.0.

@nicoloboschi
Copy link
Contributor

+1, this is blocking our integrations of nemo with other frameworks based on LangChain

@badnima
Copy link

badnima commented Jan 22, 2024

@nicoloboschi - can you please explicitly detail what you need including which LangChain version you're using (and possibly why). Thank you!

@baggiponte
Copy link
Contributor Author

@nicoloboschi - can you please explicitly detail what you need including which LangChain version you're using (and possibly why). Thank you!

I think it could be safe to move to langchain>0.1.0. The authors explicitly said it will be stable and releases will be less frequent.

@nicoloboschi
Copy link
Contributor

nicoloboschi commented Jan 22, 2024

@badnima I want to pin to a specific version of langchain which I know it works well (because I've run my application and tests on it) and I want to use it with NeMO; and the version is not the same imported by NeMo.

I think it could be safe to move to langchain>0.1.0.

Yes. I totally agree.

@drazvan
Copy link
Collaborator

drazvan commented Jan 24, 2024

Yes, let's go with langchain>0.1.0. If we see breaking changes again, we can switch back to being more conservative.

@piotrm0
Copy link
Contributor

piotrm0 commented Jan 24, 2024

The pydantic < 2 requirement is problematic for me. The bits of testing I have done suggests that it is ok to use >= 2 with nemo guardrails. Thoughts on removing the pin?

@jordanrfrazier
Copy link

Hi @piotrm0 wanted to check in on this and ask if there's any help I can provide?

@baggiponte
Copy link
Contributor Author

Yes, let's go with langchain>0.1.0. If we see breaking changes again, we can switch back to being more conservative.

Sorry I missed this. Will experiment with this later today.

@drazvan
Copy link
Collaborator

drazvan commented Jan 31, 2024

I've just tested with both v1 and v2. There were a couple of fixes needed to make the tests run (particularly the FakeLLM class used during the tests). Pushed the changes to the develop branch. We'll include this in the 0.7.0 release which is due in a few hours.

On a different note, if anyone can help with doing some additional testing on this: #288, that would be very helpful. Planning to release this as 0.7.1 as well, so we can revert just in case something breaks.

@drazvan
Copy link
Collaborator

drazvan commented Jan 31, 2024

Closing this as it is now fixed in https://github.com/NVIDIA/NeMo-Guardrails/releases/tag/v0.7.0.

@drazvan drazvan closed this as completed Jan 31, 2024
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

6 participants