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

Add type annotations #101

Merged
merged 6 commits into from May 9, 2020
Merged

Add type annotations #101

merged 6 commits into from May 9, 2020

Conversation

bmerry
Copy link
Contributor

@bmerry bmerry commented May 6, 2020

I'd sprinkled a few on to new code, but now I've gone through and done it properly. I wanted to do this now before doing async telstate, because async telstate is going to start by copying a lot of code then sprinkling asyncs and awaits on it, so it's less effort to do this first than try to add the annotations to both sync and async versions.

bmerry added 3 commits May 6, 2020 15:42
This also picked up a few bugs:
- load_from_file would try to transfer expiry times back into redis, but
  did so incorrectly (confusing absolute and relative times). This is
  not an issue in practice since we don't use expiry times.
- Calling get_range on an indexed key in a MemoryBackend would not raise
  ImmutableKeyError, and would probably crash out with some other error.
This tells mypy that katsdptelstate has type annotations.
bmerry added 3 commits May 6, 2020 16:07
mypy 0.770 was already passing, but adding some annotations for mypy
0.720.
Should be redis.client.Script - it doesn't get exported to the root
namespace.
self.ts.test_key = 'foo'
self.assertEqual(self.ts['test_key'], 'foo')
self.assertEqual(self.ts.key_type('test_key'), KeyType.IMMUTABLE)
with self.assertRaises(AttributeError):
self.ts.root = 'root is a method'
self.ts.root = 'root is a method' # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest, why do you ignore types here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mypy doesn't like one assigning an instance member that shadows a method of the class.

@@ -23,6 +23,7 @@
import threading
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this file looked new... great idea (wrong PR, I know)!

@bmerry bmerry merged commit 4a60d87 into master May 9, 2020
@bmerry bmerry deleted the mypy branch May 9, 2020 11:48
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.

None yet

2 participants