Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Consider keeping compare module level function #258

Copy link
Copy link
Closed
@Askaholic

Description

@Askaholic
Issue body actions

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

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

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.