toolbelt
toolbelt copied to clipboard
requests
Avoid loading all modules by removing imports in __init__
I want to use only a very small subset of requests-toolbelt, as I imagine most users do. What do you think about removing the shortcut imports from requests_toolbelt/__init__.py to avoid loading the rest? Right now it probably makes no difference performance-wise, but I think it would enable requests-toolbelt to include more "heavyweight" functionality (e.g. things that load additional C-extensions) without any performance hits in the future.
- #55
So there aren't a whole lot of imports in __init__ right now. Removing them would mean having to release 1.0 because the short-cut imports are part of the defined API.
Further, if there were any "heavyweight" additions to the toolbelt, I would default to not automatically importing them (since that's far more reasonable to expect). I'll leave this open but it likely won't happen for a very long time as we're not even close to a 1.0 release.
You don't need to release 1.0 for this: semver states that API changes are freely allowed in 0.X.Y
It's been a while but changing from
from requests_toolbelt import MultipartEncoder
To
from requests_toolbelt.encoder import MultipartEncoder
Is a breaking API change. Users upgrading from 0.3 to 0.4 using the documented API will encounter breakage in the form of import errors. Jow is that allowable under semver?
You don't need to release 1.0 for this: semver states that API changes are freely allowed in 0.X.Y
That's only half of the story: Yes, API changes are allowed, in the sense that you are allowed to add new functionality. This doesn't apply here.
From the website:
(4) Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.
Note also the next point:
(5) Version 1.0.0 defines the public API. The way in which the version number is incremented after this release is dependent on this public API and how it changes.
I don't think we can apply this to Python packages. I've seen many packages that follow semver in every sense, except they start to maintain backwards compat from 0.x on.
Realistically, it entirely depends. You can apply semver here, or not. All that matters is that you take a position. Requests, for example, followed this position for 0.X.Y.
Realistically, it entirely depends. You can apply semver here, or not. All that matters is that you take a position. Requests, for example, followed this position for 0.X.Y.
Right. Here's the thing, I value correctness (and I think @untitaker's idea is the correct approach since this library is a collection of curated resources, not really a unified library that does a thing) but I also value user/developer experience. I know SemVer warns users that v0.* is unstable, and that's fine. I personally interpret the warning to be more of a warning about what can happen when v1.0 is released, not when v0.x and v0.y (for x < y) happens. I think 0.* should be a series of backwards compatible releases and that as a package nears 1.0 deprecation warnings should be issues wherever practical.
For example, to begin work on this issue, now, the documentation (readme, docs, doc-strings) should all be revised soon-ish to reflect what the API will be in 1.0 because in all likelihood, the structure of the library will not change drastically between now and 1.0, what will change is how things like SSLAdapter and MultipartEncoder are imported from the library. We can essentially tell new users how to use the library in a forward-compatible way, so that when 1.0 comes, they'll be prepared. We can also start issuing DeprecationWarnings (probably through subclassing) for objects we import so they can be used as requests_toolbelt.MultipartEncoder. This will ideally help existing users move towards rewriting imports to be forwards compatible.
Make sense? (FWIW, this is my position on all of my libraries)
In terms of API "correctness" I think there's nothing wrong with providing shortcuts. The problem I see (in the long run) is purely from a performance perspective. I.e. if Python supported something like "lazy imports" natively for this purpose (i.e. without leaky magical import hooks), I wouldn't have opened this issue in the first place.
But whatever your motivation is, I'm happy that you are willing to do this change. :)
Correction: "Correctness" as defined by my skewed idea of types of libraries, their declared purpose, and their function. ;)
We can also start issuing DeprecationWarnings (probably through subclassing)
I don't think that's a good idea, you would get different objects when importing from the new vs the old location. Werkzeug has an import hook that would work well for this purpose (although it has many problems, which is why we want to remove shortcuts for 1.0 there too)
Unlike the Werkzeug approach, we could also add an import hook that doesn't actually find the correct object to import, but only spits out the warning.
I don't think that's a good idea, you would get different objects when importing from the new vs the old location.
You're correct but I'm having trouble imagining a case where this would affect users. Either way, it's merely a possibility, not necessarily something that we have to do. :)
@untitaker can you open a separate issue for the documentation bits. I'll mark it as low-hanging fruit so someone can come along and tackle that at their convenience.