Type hints!

[drewj-usnctech]

Pulling out a thread from ARMI 1.0.0 pathway to talk about type hints in Python!

Not intending for this to force the dev team one way or another, but felt like a good way to get a sense for what devs and users want

Full numpydocs

No use of type annotations in signatures. Putting all the type information in the docstring, e.g.,

def add(a, b):
    """Add two things

    Parameters
    -----------
    a : float
        First thing
    b : float
        Second thing

    Returns
    --------
    float
        sum of a + b
    """"
    return a + b

Pro: Tools like sphinx for documentation pick this up super well. You can add descriptive information about parameters and returned objects in your docstring. And in most cases you're probably writing a docstring already

Con: IDEs and type checkers do not parse docstrings so you don't get any nice typed information when developing. In cases like VSCode, you can hover over a function to see the docstring, but the returned object usually has a type of Any because the type-checker doesn't know what was returned

Full type annotations

Put type hints right in the signature, e.g.,

def add(a: float, b: float) -> float:
   """Add  two things"""
   return a + b

Pro: IDEs and type checkers parse this so you'll get typed information when developing. Sphinx appears to handle signature annotations with the autodoc extension

Con: You're probably going to still want to write a docstring for some or most cases (e.g., purpose of the class, class attributes, exceptions raised, notes, etc.) so then do you exclude documenting parameters and return values? What if you want to add context to a variable?

Both?

Eat the duplicity because type checkers in IDEs are useful but also types and explanations in documentation is also useful. But which is more useful?

Stub files?

Generate and ship some stub files with the repo? #531 used monkeytype on a small set of the code base, so there's a precedence?

[albeanth]

One additional point for type hints + type checkers (and is a big plus, IMO). You get the ability to add type checking to CI.

• mypy
• pyright

[john-science]

That could be cool. But, will these work if only a portion of our codebase has type hints?

I'm just thinking: we have hundreds of thousands of lines of code, getting 100% type hint coverage will take a long time. In the mean time, we don't want these type checkers to fail. So, if they can work with partial type hints, that will be 99% more useful to us.

I'm just thinking the logic in a type checker of method A depends on the types of the methods B and C that call method A, you know? Like, even if the type checkers don't fail if methods B and C aren't typed, the type checker might not have enough meaningful information to work with.

[john-science]

Being able to automate stub generation sounds pretty cool.

Though, I would be money that's not going to annotate 100% of ARMI automatically.

[albeanth]

That could be cool. But, will these work if only a portion of our codebase has type hints?
I'm just thinking: we have hundreds of thousands of lines of code, getting 100% type hint coverage will take a long time. In the mean time, we don't want these type checkers to fail. So, if they can work with partial type hints, that will be 99% more useful to us.

I agree, that it will take a long time to get full coverage (likely a very long time). I believe that a graded approach will work. I've testing this locally so that only a single file is subject to type checking. So I believe we could gradually add files/packages/etc. I have not tested this with CI, however.

I'm just thinking the logic in a type checker of method A depends on the types of the methods B and C that call method A, you know? Like, even if the type checkers don't fail if methods B and C aren't typed, the type checker might not have enough meaningful information to work with.

I imagine there will be many corner cases for us to deal with. I think it should be ok in this situation (the return types of methods B and C, being unknown, would have nothing enforced, but I could be mistaken).

To make matters more complex, it may depend on which type checker you use (there are several choices).

[albeanth]

Being able to automate stub generation sounds pretty cool.
Though, I would be money that's not going to annotate 100% of ARMI automatically.

Agreed. It very likely would not work automagically, especially given all of the custom types. But it might provide a good starting point for some packages/files.

[john-science]

@drewj-usnctech ...

def add(a: float, b: float) -> float:
   return a + b

This is not sufficient. Nuclear code is complicated, we aren't building website; we need docstrings that describe things in words. I'm afraid I couldn't get something like this past QA or management.

So, we should change this example to also include some sort of docstring.

[drewj-usnctech]

Totally agree. Not trying to advocate for removing docstrings. I tried to communicate this in

Con: You're probably going to still want to write a docstring for some or most cases (e.g., purpose of the class, class attributes, exceptions raised, notes, etc.) so then do you exclude documenting parameters and return values? What if you want to add context to a variable?

So I guess the question is what is the source of truth for the type information: docstring or signature?

[albeanth]

We definitely, without a doubt, need docstrings.

So I guess the question is what is the source of truth for the type information: docstring or signature?

My preference would be within the signature. This would enable type checking which will likely help find bugs and generally help improve the structure of the code (though admittedly, getting there to 100% completion is a massive lift).

I did a little toying and found that having the types in the signature still retain nice IDE functionality. E.g. by making this change:

you can still see type and docstring information on a hover in vscode

The docs build is also similar. The types live on the signature and the description lives in the rendered docstring.

[drewj-usnctech]

Casting my vote for preferring type hints in signatures. As someone who builds off of ARMI heavily, having my IDE know the type of thing returned from a function/method is immensely valuable. Otherwise I need to find the function in the documentation and follow that rabbit trail.

But, as someone who struggles with balancing type hints and docstring type information, I feel the pain of the duplicity. Especially when you want to add commentary to a input parameter, you're going to want to document that somehow.

[albeanth]

I will also vote for types in the signatures. I think the long term gains of enabling type checking will be great. And it shows to regulators and user/dev communities that we're making an effort to stay with the state-of-the-art best practices.

However, I will also say that using stub files in definitely interesting! I just don't have any experience with them. If they can also accomplish the goal of leveraging CI for type checking then that's a win in my book.

[opotowsky]

I voted numpy for v1.0.0

After that, y'all do whatcha wanna...I'll review the PRs!

[john-science]

@jakehader has proposed another option: beartype.

[keckler]

Seems interesting. Also seems very ugly to include a decorator on every single method :-\

I'd be very curious to see the actual overhead. It'd be nice if there was an option to turn it off in production runs.

[jakehader]

You don't have to include a decorator to my understanding, that's just an option.

[andfranklin]

My vote is for a combination of docs & hints for this phase of development. Encourage developers to add type hinting, but don't force it.

It's been my experience that type hinting can be a powerful tool for documentation. It can also help guide good code design, and improve the development experience.

Type hinting can also be a rabbit hole where the benefits of a duck typed language vanish (i.e., rapid prototyping and quick development). In many cases, it makes more sense to commit to a compiled language with strong typing after a certain point. Especially when performance is a valuable design objective.

Echoing what @john-science said, I've heard automatic stub generation isn't a silver bullet. There will be additional work to get 100% typing. Not to mention, the continued effort to maintain it. Also, it'll add more work to integrate type checking into the CI. None of these things are necessarily bad, they're just things to consider.

[john-science]

Well, we have a reality check coming.

I have tried to implement type hinting in ARMI. Unfortunately, this will require a near total rewrite of the ARMI API. The problem is...

Circular Imports

The ARMI API was written without type hints, so adding them means re-writing the API to prevent circular imports. Upon my first try, I found that adding type hints to 1000 Python methods resulted in ~100 separate circular imports that need to be fixed. This would mean reorganizing the entire codebase, and rewriting a large chunk of it. This would be the mother of all API-breaking changes for our downstream users.

Sigh.

If I'm going through the exercise of rewriting the API to that level, I would prefer to just rewrite ARMI in Rust.

[albeanth]

Can you provide an example of one (or some) of the circular imports you found?

[opotowsky]

The circular import bugs were found probably 2 years ago. Not sure if we documented in a ticket or a PR

[albeanth]

The circular imports were all resolved I thought? Or maybe that's one of the internal ARMI apps we have and ARMI is still a bowl o' spaghetti?

[andfranklin]

I'm on team RARMI!

Although... I don't want to do a simple re-write. I'm thinking a re-design needs to happen first 😉

[john-science]

Can you provide an example of one (or some) of the circular imports you found?

I mean, they aren't interesting, they are just circular imports.

• Case takes an input argument of type CaseSuite, but a CaseSuite is a collection of Case objects.
• A Core is a child of a Reactor, but we use Reactor numerous in the Core class.
• The Block class needs to know about BlockBlueprint class and vice versa.
• The Reactor class needs to know about various blueprints, and vice versa.

None of the above are circular imports right now. But the moment you add an import to get the typing working, these suddenly ALL become circular imports.

But, and this is the important thing, the design as it stands for any of the above is fine. They are NOT crazily, poorly designed. They were just designed before type hints existed. If unraveling all 100 of these made the code cleaner or better generally, it would feel more "worth it" to me.

TLDR; I'm not coming down hard in either direction, but right now, today, I feel like adding type hints would be months of refactoring with very little gain other than type hints. (BUT maybe I'm just in a mood and I'll feel differently later. Never say never.)