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

Why is Faker a required dependency? #304

Closed
OutOfFocus4 opened this issue Jan 8, 2021 · 8 comments
Closed

Why is Faker a required dependency? #304

OutOfFocus4 opened this issue Jan 8, 2021 · 8 comments

Comments

@OutOfFocus4
Copy link

I installed the most recent version of pylint-django, and it installed the faker package. It doesn’t look like pylint-django uses faker anywhere, so why was this dependency added?

@carlio
Copy link
Collaborator

carlio commented Jan 8, 2021

It's a dependency of astroid which is a dependency of pylint which is a dependency of pylint-django, and there was some kind of conflict - see #300

@atodorov
Copy link
Contributor

atodorov commented Jan 8, 2021

@carlio thinking about it though @OutOfFocus4 got some point. We should have probably pinned Faker only in CI and not force it onto users.

@jpulec does installing newer Faker versions when running a released pylint-django cause the same crash we saw it the tests or no? That is is there any option to pin the version only in our own CI or not ?

FTR I still haven't found time to investigate the underlying issue and chase this upstream.

@OutOfFocus4
Copy link
Author

I have astroid 2.4.2 installed on my local machine, and running pip show astroid indicates that its dependencies are six, lazy-object-proxy, typed-ast, and wrapt.

@carlio
Copy link
Collaborator

carlio commented Jan 8, 2021

Using pipdeptree, Faker comes from the factoryboy dependency, which we only use for tests, therefore @atodorov I think we could just move the Faker dependency into the extras_require[for_tests] section.

@jpulec
Copy link
Contributor

jpulec commented Jan 8, 2021

@atodorov It's only a problem for tests. As @carlio has recommended, it should instead get moved to extras_require[for_tests].

@carlio
Copy link
Collaborator

carlio commented Jan 8, 2021

@atodorov @jpulec I made a PR (#305 ) to sort this out, I'll leave it open to give you a chance to review but as it's quite small, I'll release it tomorrow as a minor version if there are no objections.

@atodorov
Copy link
Contributor

Fixed in v2.4.2, pushed to PyPI just now.

@carlio
Copy link
Collaborator

carlio commented Jan 10, 2021

@atodorov oops, thanks for releasing I had forgotten to do it :-)

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

4 participants