Improve version checking

[ProGamerGov]

Without the packaging library, statements like: "1.8.0" > "1.10.0" will be equal to True, despite v1.10 being a later version that v1.8.0.

The packaging library will in some cases not be already installed on a user's device, so I've also added it the setup.py file. It's one of the core libraries from the Python Packaging Authority, but it's not included with the base Python installation: https://packaging.python.org/en/latest/key_projects/#pypa-projects

This wasn't an issue in https://github.com/pytorch/captum/pull/940 as one the libraries in dev install has packaging as a dependency. So, there's no error when the tests are using the packaging library.

[NarineK]

@ProGamerGov if you rebase from master #1000 will hopefully fix failing conda issue

[ProGamerGov]

@NarineK It worked on this PR, but I tried it on #985 and it's still running after 3 and half hours.

Edit: It finally passed after just under 4 hours.

[NarineK]

@ProGamerGov, would it be possible to use something like this:

def versiontuple(v):
    return tuple(map(int, (v.split("."))))

Especially because pytorch's versioning is non-complicated and follows the same pattern. We usually avoid adding new dependancies to Captum if there is another possible solution without additional dependancies. I understand the advantage of package over tuple comparison but in this case pytorch's versioning is pretty straightforward and non-complex.

https://stackoverflow.com/questions/11887762/how-do-i-compare-version-numbers-in-python

[ProGamerGov]

@NarineK That solution fails for nightly builds:

def versiontuple(v):
    return tuple(map(int, (v.split("."))))

output = versiontuple("1.12.0.dev20201109")
print(output)

---------------------------------------------------------------------------

ValueError                                Traceback (most recent call last)

[<ipython-input-2-32fcc066bf69>](https://localhost:8080/#) in <module>()
      4 
      5 
----> 6 output = versiontuple("1.12.0.dev20201109")
      7 print(output)

[<ipython-input-2-32fcc066bf69>](https://localhost:8080/#) in versiontuple(v)
      1 def versiontuple(v):
----> 2     return tuple(map(int, (v.split("."))))
      3 
      4 
      5 

ValueError: invalid literal for int() with base 10: 'dev20201109'

Though I suppose we could find a way to make it work. Are the other PyTorch libraries like Torchtext using the same versioning pattern?

The packaging library is also already essentially a package dependency of Captum as it is installed as a dependency during the dev install, which is why I'm suggesting it.

Edit:

This might work for PyTorch versions (and hopefully other PyTorch library versions as well):

from typing import Tuple

def parse_version(v: str) -> Tuple[int, int, int]:
    """
    Parse version strings into tuples for comparison.
    """
    v = v.split(".")
    v = [n for n in v if n.isdigit()]
    v += ([0] * (3 - len(v)))
    assert len(v) == 3
    return tuple(map(int, v))

output = parse_version("1.12.0.dev20201109")
print(output)

[ProGamerGov]

@NarineK Is captum/_utils/common.py where we'd add the version parsing function?

from typing import Tuple, Optional

def _parse_version(v: str, length: Optional[int] = 3) -> Tuple[int, int, int]:
    """
    Parse version strings into tuples for comparison.
    
    Versions should be in the form of "<major>.<minor>.<patch>", "<major>.<minor>",
    or "<major>". The "dev", "post" and other letter portions of the given version will
    be ignored.
    
    Args:

        v (str): A version string.
        length (int, optional): The expected length of the output tuple. If the output
            is less than the expected length, then it will be padded with 0 values. Set
            to None for no padding or length checks.
            Default: 3

    Returns:
        version_tuple (tuple of int): A tuple of 3 integer values to use for version
            comparison.
    """
    v = [n for n in v.split(".") if n.isdigit()]
    if length is not None:
        v += ([0] * (length - len(v)))
        assert len(v) == length
    return tuple(map(int, v))

[ProGamerGov]

@NarineK I've implemented the _parse_version function and supporting tests.

[NarineK]

@ProGamerGov, thank you for making the changes. I was looking into the dependency graph of the libs and I noticed that mathplotlib already depends on packaging if that's the right packaging library that I'm looking into.

Is there need to add it here ? We already have the dependency in the non-dev setting as well ?

install_requires=["matplotlib", "numpy", "packaging", "torch>=1.6"],

https://github.com/matplotlib/matplotlib/blob/bfec1db0d595ccd1e14cccc0473a50a90ce26a8a/lib/matplotlib/testing/decorators.py#L12

[ProGamerGov]

@NarineK I just tested installing Captum & Matplotlib, and neither installed the packaging library. Its listed in Matplotlib's setup.py, but not their setupext.py (which I think is used for the general user version of the package). They also use it for testing and some of the optional modules it looks like: https://github.com/matplotlib/matplotlib/search?p=1&q=packaging

[NarineK]

@NarineK I just tested installing Captum & Matplotlib, and neither installed the packaging library. Its listed in Matplotlib's setup.py, but not their setupext.py (which I think is used for the general user version of the package). They also use it for testing and some of the optional modules it looks like: https://github.com/matplotlib/matplotlib/search?p=1&q=packaging

Thank you for checking @ProGamerGov, makes sense! I added a question related to the _parse_version function ?

[ProGamerGov]

@NarineK I've answered your question! The packaging library is downloaded as a dependency in the Captum dev install, but I'm not sure which package uses it as a dependency. So, we only need the _parse_version function for non test related code

[facebook-github-bot]

@NarineK has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.