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

(Partially) add type hints. See #309. #391

Merged
merged 2 commits into from
Jan 20, 2020

Conversation

AdamHillier
Copy link
Contributor

No description provided.

Copy link
Contributor

@jneeven jneeven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing the lord's work 🙌

Copy link
Member

@lgeiger lgeiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow that's a lot of type hints 🎉
This really makes me miss the Typescript type system though 😉

I only have one minor comment:

return tf.clip_by_value(x, -self.clip_value, self.clip_value)

def get_config(self):
def get_config(self) -> Dict:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General Python typehints question: Is using the builtin dict equivalent to typings.Dict in this case?

Suggested change
def get_config(self) -> Dict:
def get_config(self) -> dict:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure about the difference, but now I look into it apparently it's better to use typing.Mapping for such things, so I will edit.

larq/models.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-improvement Internal Improvements and Maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants