Type checking with pyright

[mccalluc]

• Fix Add type info #13
• Remove mypy and add pyright config to pyproject.toml
  • Turn on strict mode, and ignore the templates in no-test
• Fix many warnings, and add many ignores.
  • ~60 usages of # type: ignore
  • ~30 of these are on server functions which are flagged as unused, because Shiny is matching them with strings.
  • For the rest, I do not think it makes sense to try to resolve every last one of these right now:
    • Pyright's docs emphasize that the addition of typing should be incremental. It doesn't need to be all or nothing.
    • Adding static checks incrementally and gradually raising the bar has worked well for me on other projects.
    • If we just wanted not to have type: ignore, we could drop strict, but I think the reminders are good.
    • There are diminishing returns.
• Improvements:
  • Typing that distinguishes reactive values from plain values seems particularly useful, and probably is better than inventing a naming convention.
  • The weights are different from others, because the user is selecting a string, which we need to then explicitly cast to a number. Typing makes this more explicit.
  • Templates are now stringified with a call to .finish(), rather than str(); The old way would require me to implement a more complicated interface to really comply with the typing.

My plan is to continue this work incrementally, but I think having the checks in place is enough for now. I would particularly like to clean up a couple places where dicts could be replaced with NamedTuples.

For the reviewer:

• Do the types improve the readability/maintainability of the code for you?
• Does this seem like a good place to commit to main, or are there particular type: ignore that you think could use more attention?
• Does the switch from mypy to pyright make sense?

[mccalluc]

After sitting with this, I think my preference would be to remove strict, and then most of the overrides could be removed, but it would be good to get someone else's opinion. That way, we would get the benefit of the checks where they are supplied, and it would be up to us to keep adding them on new code going forward.

[ChengShi-1]

After sitting with this, I think my preference would be to remove strict, and then most of the overrides could be removed, but it would be good to get someone else's opinion. That way, we would get the benefit of the checks where they are supplied, and it would be up to us to keep adding them on new code going forward.

I agree with opting for non-strict mode, as this doesn't appear to be a complex application. Removing strict could make the codebase cleaner and enable faster development while still let us to selectively apply strict checks where needed.

[ChengShi-1]

I visually inspected the changes for type checking, everything looks good to me.

The types are good and improve the readability for sure. Maybe more discussion about the strict mode to the team? Also, it seems like a bunch of warning happened because the Reactive.Value[] type was not equal to primitive type even though they are both int/float/other.

[mccalluc]

Let me merge main into this and we can go from there.

Also, it seems like a bunch of warning happened because the Reactive.Value[] type was not equal to primitive type even though they are both int/float/other.

These reactive.values do need to be treated differently from the types they wrap, and having a warning about these is actually one of the big wins. To get the wrapped value from a reactive.value, you need to call it as a function:

>>> name = reactive.value('chuck')
>>> name
<function name at 0x...> or something like that
>>> name()
'chuck'
>>> name.set('not chuck')
>>> name()
'not chuck'

(This is not an actual repl session: Reactive values don't work outside of a reactive context, so this wouldn't work.)

[mccalluc]

Tidied up after merge from main, and applied typing with the nice NamedTuples that were introduced in the refactoring of code generation. Merging this much will be a big improvement, and we can still make incremental improvements.