Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-31000

Exclude version.py from mypy in ts_salobj and ts_tcpip

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Story Points:
      1
    • Epic Link:
    • Sprint:
      TSSW Sprint - Jul 05 - Jul 19
    • Team:
      Telescope and Site
    • Urgent?:
      No

      Description

      CI is failing for ts_salobj (and probably ts_tcpip) because the version.py file is built before it runs mypy, and mypy doesn't like the file.

      It's probably worth updating the version.py file to pass mypy, but that can be a bit tricky (especially since mypy changes from version to version, and finding a version that works with our pinned mypy and DM's may take some effort). But fundamentally I think I should configure mypy to ignore that file as we don't care about type annotations in it.

        Attachments

          Activity

          Hide
          rowen Russell Owen added a comment - - edited

          This turned out to be rather complicated. mypy has an exclude option, but it only affects automatic discovery. This has several limitations:

          • If a module is imported by another module then it is still checked, even if listed in exclude. This is a know mypy misfeature.
          • pytest specifies each file to test, and this again overrides the exclude (which is not surprising).

          To fix the first problem I changed __init__.py to not import version.py if type checking. It's ugly, but necessary.

          To fix the second problem I added --ignore-glob=*version.py to pytest addopts in setup.py.

          I also added exclude = version\.py to the mypy section. It is not needed to run pytest, but helps if anyone wants to run mypy from the command line.

          Perhaps we should make version.py mypy-compliant, but fundamentally I think we should not be checking version.py, especially mypy is a moving target; code that passes at one time may fail for a newer version. We can pin the version of mypy we use, but it's much easier to coordinate changes within a package to the pinned version of mypy than it is to track changes to the code that generates the version.py file.

          Pull requests:

          Show
          rowen Russell Owen added a comment - - edited This turned out to be rather complicated. mypy has an exclude option, but it only affects automatic discovery. This has several limitations: If a module is imported by another module then it is still checked, even if listed in exclude . This is a know mypy misfeature. pytest specifies each file to test, and this again overrides the exclude (which is not surprising). To fix the first problem I changed __init__.py to not import version.py if type checking. It's ugly, but necessary. To fix the second problem I added --ignore-glob=*version.py to pytest addopts in setup.py. I also added exclude = version\.py to the mypy section. It is not needed to run pytest, but helps if anyone wants to run mypy from the command line. Perhaps we should make version.py mypy-compliant, but fundamentally I think we should not be checking version.py, especially mypy is a moving target; code that passes at one time may fail for a newer version. We can pin the version of mypy we use, but it's much easier to coordinate changes within a package to the pinned version of mypy than it is to track changes to the code that generates the version.py file. Pull requests: https://github.com/lsst-ts/ts_salobj/pull/200 https://github.com/lsst-ts/ts_tcpip/pull/7
          Hide
          rowen Russell Owen added a comment -

          Reviewed on github

          Show
          rowen Russell Owen added a comment - Reviewed on github
          Hide
          rowen Russell Owen added a comment - - edited

          Released ts_salobj v6.5.1 and ts_tcpip v0.3.1

          Show
          rowen Russell Owen added a comment - - edited Released ts_salobj v6.5.1 and ts_tcpip v0.3.1

            People

            Assignee:
            rowen Russell Owen
            Reporter:
            rowen Russell Owen
            Reviewers:
            Wouter van Reeven
            Watchers:
            Russell Owen, Tiago Ribeiro, Wouter van Reeven
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.