Skip to content

Consider keeping compare module level function #258

Description

@Askaholic

I would think that comparing two semver strings is a pretty common use case and this code:

semver.compare("1.2.3", "3.2.1")

is arguably a lot nicer than

semver.VersionInfo.parse("1.2.3").compare("3.2.1")

plus it is probably already present in a large number of projects that use this library, so removing it entirely should be done only if absolutely necessary (which I do not believe that it is).

The fact that VersionInfo.compare accepts the other argument as a string says a lot about the convenience and desirability of working with strings. But it leaves us in this weird situation where VersionInfo.compare takes 2 arguments (self, and other) where self must be a VersionInfo object but other can be one of many types. If we were to write out the type signature we would have something like:

VersionInfo.compare(self: VersionInfo, other: Union[str, dict, list, tuple, VersionInfo])

(Another quirk of this function is that given a tuple as an argument, it will convert to VersionInfo and then immediately back to a tuple again.)

I think as far as API design goes, it doesn't make a lot of sense for the main comparison function to have this type signature. Why is only one argument required to be a parsed VersionInfo object? Why are we doing automatic conversion on one argument but not the other? What if I already have both arguments in tuple form and don't want to perform two useless conversions?

I would suggest having the module level API be a sort of convenience wrapper around the VersionInfo API. I think it is likely that the vast majority of semver data starts out in string form, so there should be a quick and clean API that one can call on this data. I would suggest making semver.compare look like this:

semver.compare(left: Union[str, dict, list, tuple, VersionInfo], right: Union[str, dict, list, tuple, VersionInfo])

And I would probably avoid double conversions on lists and tuples.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    EnhancementNot a bug, but increases or improves in value, quality, desirability, or attractivenessQuestionUnclear or open issue subject for debateRelease_3.x.yOnly for the major release 3

    Type

    No type

    Fields

    SIze

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions