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

Version 4.2.3 breaks other packages #225

Closed
tzickel opened this issue Sep 30, 2021 · 9 comments
Closed

Version 4.2.3 breaks other packages #225

tzickel opened this issue Sep 30, 2021 · 9 comments

Comments

@tzickel
Copy link

tzickel commented Sep 30, 2021

I do not know if such a minor release has the intended consequences of breaking the API contract, but here is a project that just stopped working because of it:

https://github.com/wimglenn/johnnydep/blob/8c3f1e9eb81bc43db3b5f9d1f0b4b3a0c187eb82/johnnydep/pipper.py#L21

I assume this commit is at fault:
be507a6

@wimglenn

@wimglenn
Copy link
Contributor

wimglenn commented Sep 30, 2021

Oof, that is quite a backwards break for a micro bump (v4.2.2 -> v4.2.3) in what I assumed was semver

The fix for users is easy (change their import statements) but maybe this release should be yanked and compat shims / deprecation warns put in.. ?

@tkem
Copy link
Owner

tkem commented Sep 30, 2021

cachetools uses semantic versioning, and the minor version change to 4.2.3 IMHO correctly indicates that the public API did not change.

The cachetools.ttl submodule was an undocumented implementation detail. Documentation including code examples use from cachetools import TTLCache, and have done so from the start. Why johnnydep chose to use cachetools.ttl I don't know, but this was never officially supported, nor even mentioned in the documentation.

@tkem tkem closed this as completed Sep 30, 2021
@wimglenn
Copy link
Contributor

wimglenn commented Sep 30, 2021

Yeah, my bad - the autodoc always mentioned cachetools.TTLCache. Hyrum's law bites.

It might be that some IDEs which automatically write your import statements just take them from the module where the class is defined, not sure? There is sort of a convention in Python that anything without a leading underscore is fair game. Here are ~100 others making same mistake.

I'll note that I did mention those supposed "existing APIs" in comment #146 (comment)

@tkem
Copy link
Owner

tkem commented Sep 30, 2021

Well, sorry for the inconvenience - I didn't think this would have much of an impact, but I guess you may be right about IDEs doing autocompletion. OTOH, the vast majority of users seem to do it right, and AFAICS most of the affected repos use copies/variants(?) of same code base, so who knows...

I usually do follow the underscore (or for class members, double underscore) convention, except for "private" submodules. But those were just a habit of mine, coming from languages like C/C++, and I'm slowly giving up on that, since they tend to have undesired effects.

And yes, I completely missed your comment in #146, sorry for that!

@tkem tkem reopened this Sep 30, 2021
@tkem
Copy link
Owner

tkem commented Sep 30, 2021

Temporarily reopened so others, who are affected by this, will find this more easily.

@ran-isenberg
Copy link

It broke our usage too. You should either support it backwards or change it to v5 as it's a breaking change.

@tkem
Copy link
Owner

tkem commented Sep 30, 2021

Apparently @wimglenn was right about Hyrum's law... I guess I'll add some submodule shims for backward compatibility, but please do fix your imports ASAP.

@tkem tkem closed this as completed in c234536 Sep 30, 2021
@wimglenn
Copy link
Contributor

wimglenn commented Sep 30, 2021

@tkem No big deal, but there is a slightly improved way to indicate deprecation, details in #226

@tkem
Copy link
Owner

tkem commented Sep 30, 2021

@wimglenn: Yes, of course. I know, or I knew at least... Kind of rushed this out today - thanks for pointing this out!

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