Skip to content

Conversation

nate-kean
Copy link

@nate-kean nate-kean commented May 17, 2025

Hello!

I am a long-time user of markovify, and my most recent project has involved a lot of interacting with Chain internals. I'm used to being able to know things' argument and return types in my IDE, but markovify doesn't have type annotations, and lately I've been really butting up against that.

Well, one thing led to another, and I ended up adding type annotations to the entire package.
Here is what I have, after cleaning it up a little and making sure it works in the range of Python versions I am aware that markovify supports. Please take a look.

  • All tests pass
  • Coverage is still 100%* in the business logic
  • make format returns no changes
  • make lint returns no changes
  • Tested in Python 3.6.8 and 3.13.2

The whole thing is type annotated. These are the changes made beside pure type annotations:

  • To support certain typing symbols in lower Python versions, typing_extensions is added as a dev dependency. No new runtime dependencies are added, and this is not imported at runtime.
  • *Project-wide pytest-cov code coverage decreased slightly due to the addition of some code that only runs while type checking, and an assert_never() to appease Pylance at a certain unreachable line.
  • cast_not_none() helper function added to chain.py and text.py.
  • The Nexts of a compiled model are now tuples instead of lists, in order to express that words and cff are different types and that it's always just those two items inside it. chain.compile_next()'s return type is changed accordingly, and logic was added to from_json() to convert a rehydrated, compiled model's Nexts to tuples.
    • If we decide we aren't comfortable with this and want it to remain a list, I can accommodate that.
      One alternative could be to change compiled Nexts back to a list and say it can hold either lists of ints or lists of strings, and then cast the elements as they're used (like in move()).
  • A None check was added in Chain and Text's constructors to ensure that at least one source of data is provided (e.g., that model is provided if corpus is not).
    • Previous behaviors in these spots led to runtime errors from trying to do unsupported operations on None.

nate-kean added a commit to nate-kean/conversational-markov that referenced this pull request May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant