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

Optimization in utils/wrappers.py fails to optimize

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: utils
    • Labels:
      None
    • Story Points:
      0.5
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      The TemplateMeta metaclass in utils/wrappers.py replaces the __instancecheck__ and __subclasscheck__ methods so that it can look up the type being checked against its registry of templated classes. The registry check is written incorrectly, though, checking the type against the registry keys instead of the registry values. The result is that it falls through to an expensive loop checking isinstance for each type in the values.

      The fix should be a simple addition of .values() to the if statement.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            The simple fix works but now the code coverage is degraded because the fallback code is not triggered by any of the tests.

            Show
            tjenness Tim Jenness added a comment - The simple fix works but now the code coverage is degraded because the fallback code is not triggered by any of the tests.
            Hide
            tjenness Tim Jenness added a comment -

            Kian-Tat Lim ready for review I think. The fix was trivial but now there are tests for the fallback code itself.

            Show
            tjenness Tim Jenness added a comment - Kian-Tat Lim ready for review I think. The fix was trivial but now there are tests for the fallback code itself.
            Hide
            tjenness Tim Jenness added a comment -

            Jenkins has passed.

            Show
            tjenness Tim Jenness added a comment - Jenkins has passed.
            Hide
            ktl Kian-Tat Lim added a comment -

            LGTM.

            Show
            ktl Kian-Tat Lim added a comment - LGTM.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              ktl Kian-Tat Lim
              Reviewers:
              Kian-Tat Lim
              Watchers:
              Jim Bosch, Kian-Tat Lim, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.