Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Type hinting improvements #13

Open
jamesohortle opened this issue Sep 8, 2019 · 5 comments
Open

Enhancement: Type hinting improvements #13

jamesohortle opened this issue Sep 8, 2019 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@jamesohortle
Copy link
Collaborator

@jamesohortle the code as it stands is a strong improvement to the codebase. I'm happy for you to merge into the dev branch now, and put those TODO items in an issue for enhancement in the future.
Also, the None states have a big structural importance: they serve as the pseudo 'aggregate' state, which represents the infinity of states which are not observed in the model's current state. We can't replace them completely, but maybe we can make a reserved state label so avoid the type conflicts.

Originally posted by @jamesross2 in #10 (comment)

@jamesohortle jamesohortle added the enhancement New feature or request label Sep 8, 2019
@jamesohortle jamesohortle self-assigned this Sep 8, 2019
@jamesohortle
Copy link
Collaborator Author

jamesohortle commented Sep 8, 2019

Begin the review process for the merge.

TODO:

  • Makes types more restrictive (remove all Any from .pyi and .py files, determine if types InitDict and NestedInitDict are correct).
  • Think of better names for InitDict, NestedInitDict. (ObservationSequence, HistoricalObservationSequence, ...?)
  • Provide type stubs (.pyi files) to typeshed.
  • Rethink initializations of types InitDict, NestedInitDict. (What is purpose of using None? What is purpose of pre-allocating dictionaries/lists/etc. for a dynamic language?)
    Originally posted by @jamesohortle in Add typehinting #10

@jamesohortle
Copy link
Collaborator Author

Since None is structurally important, it may be worthwhile to invest in the following idea.

Idea:

  • Create a new complex type via a class that represents an emission sequence class EmissionSequence: ... and use that as the type for InitDict, NestedInitDict, etc. Since emission sequences seem to be quite stateful objects, this will probably make their manipulation more robust (we can expose an API for these objects). Of course, this comes with its own bag of complexity which is something that should be kept to a minimum.

@jamesohortle
Copy link
Collaborator Author

Idea:

  • Write from numbers import Real and use Real as the type of the values in the sequences (avoids use of Numeric = Union[int, float]? However: PEP484 and int is not a Number? python/mypy#3186... Should I just use float instead? Seems hackey so perhaps an alias is clearer?
  • Retain the Index type.

@jamesohortle
Copy link
Collaborator Author

jamesohortle commented Sep 18, 2019

Idea: (EDIT: This is Python >=3.8 only 😭)

  • from typing_extensions import Final, final (final is decorator; use above "static" functions, classes that should not be inherited from, etc), Final is a type (like List) that you use for constants (i.e. MAGIC_NUMBER: Final = 42; no need to say int or anything since the type will be inferred).

@posita
Copy link

posita commented Dec 15, 2021

I don't know if you saw, but I pulled out a lot of my own number typing work-arounds into their own package called numerary. If this journey lines up with your own, numerary could be an intermediary solution until python/mypy#3186 is fixed. (Alternatively, the techniques used therein might provide inspiration if you can't take a dependency.) Happy to consult here, if helpful. Apologies if this is a distraction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants