Type Annotations

[eliotwrobson]

Hello,

Thanks for your work on this library! I've been using it as part of the backend for a course I'm TAing. For our purposes, we're working on adding type hints (which can be checked with mypy) to the library. We think this makes the code more readable, and easier to tell what certain functions do. Would there be interest in including these changes in this project? I'd be happy to put up a pull request once we're done.

Best,
Eliot

[caleb531]

@eliotwrobson Maybe... would you be able to guarantee Python 3.6 compatibility? I am reading that Type Annotations have had somewhat of a messy history in Python 3:

https://towardsdatascience.com/type-annotations-in-python-d90990b172dc

What are your thoughts?

[eliotwrobson]

We're working in 3.6, so compatibility shouldn't be an issue. We're not really using any super-complex features, just makes it easier to keep track of what's a set, dict, etc.

[caleb531]

@eliotwrobson Yeah, I'd say go for it! Please submit me a PR to master when you're all done. I'm guessing you won't bother to annotate the tests as well, just the core API modules, correct?

[eliotwrobson]

Yes, more than likely just the core API.

[caleb531]

@eliotwrobson Great! Then by all means, feel free to start development on this at your own convenience.

[eliotwrobson]

I've started some development on this, and I've run into a small issue. There are some internal functions that violate the written interface for the NFA file (in some places, it is assumed states must be strings, but in others, it is assumed they are integers). @caleb531 should I make changes to the files to respect the interface (and assume states are always strings)? Or is the interface incorrect?

[jasonxia17]

@eliotwrobson and I would prefer if the interface supported any hashable type as states. In the algorithms course that we TA for, it often makes sense to use integers or tuples as the states.

[caleb531]

@jasonxia17 (cc @eliotwrobson) I agree. Strings are an arbitrary choice: any hashable type should be usable for state names (or symbol names, for that matter). Furthermore, such would represent a backwards-compatible change, since strings are still supported, and the README mostly uses string-state-name examples anyway.

Eliot, I leave it to you if you'd like to declare dedicated interfaces to represent state/symbol names that can be used anywhere in the library (e.g. StateName and SymbolName interfaces, perhaps).

[eliotwrobson]

@caleb531 thanks for the clarification! I'll probably declare dedicated types that can be used across the different files. Will put up a PR once I get some things working.

I had one final question; there's a feature in Python 3.7 that makes setting up the typing a bit easier. Will you be retaining Pythen 3.6 support for this package (as 3.6 is EOL within the next week)? If you are, it's straightforward enough to do typing without the 3.7 feature.

[caleb531]

@eliotwrobson Wow, time flies. Maybe I should check the end-of-life schedule more often. 😅

Is this Py3.7 typing setup feature something more for you (the developer)? Or does it more so benefit the library user (someone who's including my lib as a dependency)?

[eliotwrobson]

@caleb531 I'm actually not 100% sure, I think it makes the type checking more robust on the user's end. It's detailed in this post.

[caleb531]

@eliotwrobson I see. I think for the time being, I would prefer to preserve 3.6 compatibility for now, then down the road, roll out the 3.7 improvement as a semver-major release.

[eliotwrobson]

Closed with discussion on PR.