HPInc / HP-Digital-Microfluidics

HP Digital Microfluidics Software Platform and Libraries
MIT License
2 stars 0 forks source link

Ensure all functions are checked by MyPy #200

Closed EvanKirshenbaum closed 5 months ago

EvanKirshenbaum commented 5 months ago

When working on #91, Rares Vernica and I ran into a situation where we were getting a None produced in what we thought was a Delayed[Drop]. It turned out that the culprit was StaticOperation.schedule(), which was defined as

    def schedule(self, *,
                 after: WaitableType = NO_WAIT,
                 post_result: bool = True,
                 ) -> Delayed[V]:
        def cb():
            self._schedule(post_result=post_result)
        return self.scheduler.delayed(cb, after=after)

The problem was that delayed() returns the result of the function wrapped in a future, so for this to be a Delayed[V], cb() would have to return a V, but it didn't. But since there were no type hints at all on cb(), MyPy ignored it and figured we knew what we were doing.

We didn't.

As soon as we added a type hint, it complained, and we realized that what we needed was

    def schedule(self, *,
                 after: WaitableType = NO_WAIT,
                 post_result: bool = True,
                 ) -> Delayed[V]:
        future = Postable[V]()
        def cb() -> None:
            self._schedule(post_result=post_result).post_to(future)
        self.scheduler.delayed(cb, after=after)
        return future

which, incidentally, was the pattern that had already been used for Operation.schedule_for().

Doing a grep, there appear to be a number of other functions without return types, some with no type hints at all.

Looking at the MyPy docs, there are a couple of flags we should give to MyPy to help us find them:

EvanKirshenbaum commented 5 months ago

This issue was referenced by the following commit before migration:

EvanKirshenbaum commented 5 months ago

Running those two arguments on the master branch results in 275 (new) errors. Hopefully most of them will be straightforward to fix.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Sep 14, 2022 at 4:28 PM PDT.