Repository navigation
Decide to deprecate module level functions? #225
Description
Situation
Currently, we have mostly functions which are either on the module level (semver.next_version, semver.bump_*) and almost the same on the VersionInfo level (semver.VersionInfo.next_version, semver.VersionInfo.bump_*).
I know, they both serve distinctive purposes: the first expects and returns a string whereas the second needs a VersionInfo object.
However, from an implementation perspective, I'm wondering if this is double work. It looks inefficient to me when the VersionInfo methods need to convert the object first into a string, call the modul level function, and convert it back to a VersionInfo object.
IMHO, we should improve this situation.
Suggested Solution
The question boils down to this: should we treat VersionInfo as first class object?
I have the impression this isn't currently the case. The developer can decide between two implementation which does mostly the same. The only difference is, one implementation use strings, the other use a "detour" with an VersionInfo object.
If we would treat VersionInfo as our one and only solution, that would mean we have to move the implementation into the methods. The module level functions would serve as "helper functions".
Or... maybe we could think about a more radical approach? Are the module level functions really necessary? Should we deprecate them? As we can convert a VersionInfo object into a string through str(versionobject), it seems to me, they are obsolete...
I see the following benefits of this approach:
- Focus on the
VersionInfoclass, all the magic would happen there. 😉 - Avoids writing two functions which mostly implements the same functionality, but with different types.
- Simplifies testing.
- Focus more on objects than strings (which can't be controlled).
- Improves the project to be more slim and well-arranged.
- Makes conversion between objects obsolete. Without conversion, it would have a improved effect on performance (maybe only slightly, but...).
If we want to follow this path, we would need:
- Use the
warningmodule to issue a deprecation warning in affected module functions. - Document the change for our upcoming major release. 😉
- Release 2.9.2 of semver to implement the warnings
- Make the whole removal in release 3 only.
@gsakkis, @scls19fr Does that make sense to you? Do I miss an important point? I would really like to hear your view on this. ❤️
Activity
gsakkis commentedon Mar 3, 2020
@tomschr I fully agree on deprecating and eventually removing the module level functions 👍
s-celles commentedon Mar 3, 2020
Removing functions on module level can only be done with major version upgrade (to respect semver). A first step could be to deprecate if there is a general consensus
tomschr commentedon Mar 4, 2020
Maybe I wasn't clear enough, but of course I meant it to remove them in a major release only. If nobody objects. ;))
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
tomschr commentedon Mar 16, 2020
Just to clarify the pull request #229: it doesn't remove the modul level function, it lays the foundation so we can remove them easily once we work on major release 3.
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
ppkt commentedon Mar 18, 2020
@tomschr I've mixed feelings about this - while I agree with your points (especially about duplicated code) - I think we should allow using semver in both ways because a lot of software is using module level functions currently.
I checked some popular tools which are using semver: https://libraries.io/pypi/semver/usage:
Enforcing OOP it in next major version is completely fine, but maybe it'll be better to put some extra effort and maintain support for current "users" of module level functions? :)
tomschr commentedon Mar 19, 2020
@ppkt Thanks Karl for your feedback, much appreciated! 👍
This is still the case with the current version. For 2.9.x, it won't change, they will be available. The only difference against previous versions are that the user get a deprecation warning message to use other functions.
Or do you talk about the next major release?
I didn't know that Web page. Very cool, thanks for the link! 👍
Ok, could you be a bit more verbose what you had in your mind?
ppkt commentedon Mar 19, 2020
Sorry, wasn't clear enough :)
My point is - a lot of software is currently using "module level functions" - by deprecating them (and removing) they might stay with 2.x version. Maybe it's better to refactor code to avoid duplication but leave both ways of working on versions (string-based and object oriented)?
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
tomschr commentedon Mar 20, 2020
@ppkt Oh, I see what you mean. So we should keep them in your opinion.
Yes, that's a valid point. I have to agree, I have mixed feelings. 😉
On one side, I can fully understand that developers don't want to change a running system. Even if the change would be minor.
On the other side, the change is not that difficult. Change is inevitable. Modules change, programs change, even big operating systems does that. 😉 Of course, the change should be communicated, useful, and there should be a clear migration path.
So... one option could be, to still have the module level functions in place for major release 3, but with all the deprecation warnings. However, I would definitely remove them in major release 4. That should give people really enough time. Of course, it should also be covered by our documentation (I currently lack a good description, I'm aware of that, but haven't had the time).
What do you think of this idea?
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
ppkt commentedon Apr 11, 2020
@tomschr I saw your comment in some other ticket and I completely agree with it - "there should be only one way to do it". I also noticed in your PR is detailed instruction how to replace deprecated functions and I think it's really great.
I also think we should stick with your initial proposal - to deprecate them in 2.9.x and remove in 3.x - there will be a lot of other, potentially breaking, changes in new version so someone who is using our library, will have to check documentation and fix all warnings and errors :)
tomschr commentedon Apr 11, 2020
@ppkt Thanks Karol for your ideas. 👍
Yes, we have now documentation for it. 😉 I've tried to address this and help our users to "migrate". If you have any further ideas, suggestions, or found anything missing, please let me know so I can improve it.
Regarding your change, yes, sure, we can deprecate them in 2.9.x and remove in 3.x. 😉 That's why we discuss this and try to find the best solution.
Could you do me a favor and can do a short test in regards to #229 (comment)? It's about the warning and it drives me nuts. 😄 Maybe I need to read the documentation more carefully.
Thanks Karol! 👍
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions
Fix python-semver#225: Deprecate module level functions