pythonfinder icon indicating copy to clipboard operation
pythonfinder copied to clipboard

Reduce/Eliminate dependencies

Open cs01 opened this issue 6 years ago • 6 comments
trafficstars

We're working on some updates to python resolution when creating virtual environments from within virtual environments with nox in https://github.com/theacodes/nox/pull/231. It was suggested to use pythonfinder rather than rolling our own python resolver, which is rather tricky as you know.

We were going to discuss it even though the nox owner is hesitant to add new dependencies to nox. Then it was noted that pythonfinder has many dependencies, and the idea was rejected outright. The number of dependencies may become a deterrent to pythonfinder adoption, so I wanted to create an issue to let you know, and perhaps consider reducing/eliminating its dependencies.

cs01 avatar Aug 06 '19 04:08 cs01

A quick review shows:

  • attrs - referenced in setup.cfg but not used
  • cached-property - referenced in setup.cfg but not used
  • crayons - only used in cli
  • click - only used in cli (secho is imported in pythonfinder but never used)
  • six - used everywhere
  • packaging - used for the LegacyVersion and Version classes, which are mostly only used in type annotations.
  • vistir - used quite a lot, but looks like only a tiny subset of the package's functionality, so pulling in everything is a lot of unneeded overhead.

Overall, it seems reasonably easy to limit the dependencies. Many of them seem like they are unused or reasonably easy to avoid. Making the CLI interface an optional extra would help (and would seem reasonable - for most people, this project will be used as a pure library, and the CLI is unneeded). Finding alternatives for the features used from vistir would be a huge saving, as well - most of the uses seem to be compatibility versions of features in Python 3, such as pathlib and lru_cache, so maybe use core features on Python 3 and conditionally depend on vistir for Python 2?

I should also point out that I'm not against depending on other libraries, but there's a balance to be had. In this instance, it was pulling in requests and all its dependencies in particular that shocked me - that plus something that we were planning on using as a pure library pulling in a command line package like click.

pfmoore avatar Aug 06 '19 17:08 pfmoore

I have much less skin in this compared to @techalchemy but I'm on board for doing this.

pradyunsg avatar Aug 06 '19 17:08 pradyunsg

I'd be willing to develop a PR for this, but (a) making the CLI an extra may impact users of the project, so I'd like confirmation that it's an acceptable change before I go ahead, and (b) I'd like to know better why vistir was chosen before I try eliminating it (mostly because I don't want to get sucked into philosophical debates about what it's OK to depend on).

pfmoore avatar Aug 06 '19 17:08 pfmoore

Sorry for the highly delayed response here -- I have no strong opinions here as long as functionality is retained. This was developed as an ecosystem library when I refactored it out of pipenv.

vistir is just a library I maintain which provides a number of useful compatibility shim for things like encodings, useful context managers, output handling, etc. that I wind up needing in most of these libraries. Most likely I refactored some things out of here and over into there. I have no issue with moving functionality back into here to reduce dependencies. Completely agree that there are a lot here.

techalchemy avatar Dec 20 '19 09:12 techalchemy

As a side note I'll be pushing release automation for this library and am happy to add any of the people in this thread as committers / maintainers etc. Feel free to let me know and I'd be happy to add some release documentation and such.

techalchemy avatar Dec 20 '19 09:12 techalchemy

@gaborbernat says virtualenv and tox will soon have code that can do this, so we might want to consider using/copying that implementation once it's landed (any updates here, @gaborbernat?). https://github.com/theacodes/nox/pull/231#issuecomment-544487046

cs01 avatar Dec 23 '19 18:12 cs01