[WIP] Annotations to help mypy

[rpgoldman]

Added comments to make mypy ignore decorated properties it doesn't understand.
Added type declaration for RNG in backwards-compatible form.

[twiecki]

Is this available in Python 2 as well?

[twiecki]

Are you going to add type hints elsewhere too?

[eigenfoo]

Just want to pitch in my two cents: adding type annotations would greatly improve the quality of life for the dev team, especially for the many (unfortunately) undocumented functions in the PyMC3 backend. It's also probably a good idea to add mypy type checks to the Travis build, if that's possible.

For now we can add them in backwards-compatible format, but when we sunset support for py2 next year, it should be fairly painless to convert all the annotations to the py3 format.

[twiecki]

For now we can add them in backwards-compatible format, but when we sunset support for py2 next year, it should be fairly painless to convert all the annotations to the py3 format.

That would be my key question, should we bother with mypy if we could just use python 3 annotations fairly soon? Master could already start becoming python 3, technically, depending on when we do the next release.

[rpgoldman]

I could be wrong, but my understanding was that the syntax is python 3.6+, but that one still needs an external tool like mypy to do the static analysis. But I'm FAR from an expert on the python ecosystem -- I just got involved to use pymc3, and I'm still learning, so you tell me.

…

Sent from my iPhone

On Sep 3, 2018, at 08:48, Thomas Wiecki ***@***.***> wrote: For now we can add them in backwards-compatible format, but when we sunset support for py2 next year, it should be fairly painless to convert all the annotations to the py3 format. That would be my key question, should we bother with mypy if we could just use python 3 annotations fairly soon? Master could already start becoming python 3, technically, depending on when we do the next release. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[twiecki]

You still need an external tool, but you can have the annotation directly in code.

[rpgoldman]

The commits I have here are pretty much all directly in the code, except I used the old way of type-hinting data properties of classes, instead of the 3.6+ way. Those would be easy to change later.

I see that I caused a problem by not commenting to hide the typing imports. I will fix this and re-push.

[rpgoldman]

If this fixes travis, and the mods are ok, probably they should be squash-merged.

[fonnesbeck]

I would hold off on type annotations until they become more standard with Py3.7 and beyond. For now, if we have undocumented functions, let’s spend the time to document them.

[rpgoldman]

@fonnesbeck The only reason I would disagree with this is that the PyMC3 code base seems to have a fairly large number of type errors in it, as witness my #3179

I don't believe that bugs like that -- which arise when code makes incorrect assumptions about the return value of the same method implemented in multiple different classes -- is readily detected by just fixing documentation.

For that matter, I wouldn't say that this is an either/or decision: I don't know enough to write good documentation for PyMC3, but I can provide some type hinting, and maybe through type hinting provide useful bug reports or patches.

P.S. My primary interest is in providing type stubs, in order to help me when I am writing code that uses PyMC3, and I think that providing those stubs is generally useful (perhaps more useful than hinting the code itself).

[rpgoldman]

@twiecki I'm trying to make sure that any hinting does not conflict with Python 2.7.
@twiecki I was starting by hinting code as I read it to deal with issues I encounter. But PyMC3 seems to tickle a bug in mypy (reported to the developers there) which makes this less appealing, because checking PyMC3 causes mypy to error out without doing any actual analysis. Making stubs for callers of PyMC3, though, seems useful, and is probably where I should focus efforts.

[fonnesbeck]

I’m not at all opposed to adding type annotations/hints, it’s a matter of when and how it happens. As Thomas suggests, it should probably happen project-wide when it does. I can make sure that the issue is discussed at an upcoming project meeting to gauge the support and timing.

[fonnesbeck]

BTW, if there are egregious cases of missing documentation, please raise an issue(s). They will be taken care of.

[rpgoldman]

@fonnesbeck For what it's worth, the biggest problem for me as a user has been inconsistency in protocols (I mean "protocols" in the common sense, not in the Python language sense). For example, in my bug report there's an assumption about what the get_values method should return, but this assumption does not seem to be shared across the project, and this creates problems. Similarly, sometimes functions want variables, and sometimes variable names are required, etc. I think those are issues that might be addressed well by static type checking. Anyway, that's what led me to trying out some type hinting and writing some type stubs. If you'd prefer not to have the type hinting, that's fine.
I think the type stubs, though, would be useful right away, because they can provide value to users of the library, and don't cause any degradation. That is the stubs they are invisible for users that don't use a tool like mypy, and can provide value to those who do.

[rpgoldman]

OK, I'm stumped by the errors in Travis. I have tried to follow the instructions I've found to tell pylinter to ignore the unused imports from typing, but that doesn't seem to work on Python 2.7. But if these type hints are premature, I'll drop it and not worry about it.

[lucianopaz]

@rpgoldman, now that we dropped python 2 support and testing, would a rebase pass on Travis?

[kyleabeauchamp]

Eventually we may want to upgrade these type hints to the python3 based syntax (https://docs.python.org/3/library/typing.html), rather than the py27-compatible syntax. The py3k syntax also seems to behave better with linters etc.

[rpgoldman]

I will check -- I haven't updated these in a while -- was just on hold. I'll get back to this sometime this week.
@kyleabeauchamp I have already been using the python 3 syntax; I don't use python 2 at all any more. That's one of the reasons I shelved this for a while. I'll see what Travis doesn't like...

[kyleabeauchamp]

In case folks didn't understand, what I mean is that we could switch from

def broadcast_message(message, serveres):
    # type: (str, Sequence[Tuple[Tuple[str, int], Dict[str, str]]]))-> None 
    ...

to

def broadcast_message(
        message: str,
        servers: Sequence[Tuple[Tuple[str, int], Dict[str, str]]]) -> None:
    ...

[twiecki]

Yeah I think it'd be great to make use of that python 3 feature now that we can. I'll close this but feel free to keep posting.

[rpgoldman]

@twiecki Would it be reasonable to reopen this, and retitle it as a WIP merge request? Then the mainline of the project can track as I develop more annotations, while it's still too early to be merged.

It looks like when you close this, the Travis checking is removed. If we want to keep this closed, can you clue me in about how I can put Travis checks in place on my branch?

[twiecki]

Sure, if you prefer to keep developing on this PR.

[rpgoldman]

Looks like switching to Python 3 exclusively solved the problems I was having with test failures. That's nice!

How do the developers feel about type annotations in PyMC3 itself?

[ColCarroll]

I don't have much experience with mypy, but my impression is that:

1. They can be introduced gradually
2. Those that exist can be enforced via a check on CI
3. They are very helpful for developer productivity (if their editor is set up to use them, i.e., pycharm)
4. They are very helpful for enduser productivity (again, if the editor is set up)

Do you know if these 4 sentences are true, or would you rewrite them or add caveats?

If they were true, I would suggest we start allowing type annotations, see how helpful it is for devs and users, and then make a more focused push if they were, indeed, very helpful.

[rpgoldman]

I don't have much experience with mypy, but my impression is that:

1. They can be introduced gradually
2. Those that exist can be enforced via a check on CI
3. They are very helpful for developer productivity (if their editor is set up to use them, i.e., pycharm)
4. They are very helpful for enduser productivity (again, if the editor is set up)

Do you know if these 4 sentences are true, or would you rewrite them or add caveats?

If they were true, I would suggest we start allowing type annotations, see how helpful it is for devs and users, and then make a more focused push if they were, indeed, very helpful.

I think that's mostly true. I'd add a couple of points:

1. One can separately make type stubs that will be useful to end-users. This may be the best way to get value for end users, and although it involves a bolus of work, it can be relatively small in scope, because all one needs to do (to a first approximation) is to annotate the API.
2. The typing information has some holes in the value it provides, because as an end user I find that many of my problems actually happen in interactions with numpy objects, and unfortunately (a) there aren't good numpy type stubs and (b) the python typing system isn't capable of things like checking array shape constraints.

If there's interest, it might help to introduce, at least for end users, some explicit protocol notions. For example, IIRC MultiTrace objects, the results of pre-posterior samples, and posterior_prospective sampling all have some features in common, but as a programmer it's not obvious what is and isn't common in interacting with them. This can, e.g., make plotting difficult.

[twiecki]

I'd be very interested in seeing a PR that shows this. probably good to start small

[twiecki]

Closing due to inactivity.