Add definitions for boto/utils.py #1579

mxr · 2017-08-28T22:29:30Z

No description provided.

JelleZijlstra · 2017-08-29T04:12:13Z

third_party/2and3/boto/utils.pyi

@@ -0,0 +1,3 @@
+from typing import Text
+
+def pythonize_name(name: Text) -> Text: ...


There's a lot of other code in this module. We generally shouldn't create a partial stub, because then users who use other functions from this module will get false positive errors that are hard to ignore. (If the module doesn't exist in typeshed at all, mypy's --follow-imports flag lets you shut up mypy.) Would you mind adding stubs for the other public APIs in boto.utils?

mxr · 2017-09-05T20:45:16Z

@JelleZijlstra I'm almost done adding stubs here but I hit a roadblock. Any advice on the below errors?

third_party/2and3/boto/utils.pyi:162: error: Invalid type "boto.utils.Password.str"

The Password class has a variable called str which mypy hiccups on. Should I remove the str variable from the stub or do something else?
third_party/2and3/boto/utils.pyi:1: error: Cannot find module named '_thread'

I'm trying to annotate with LockType from _thread (which exists in python2 and python3) but mypy isn't picking it up. Should I split my stubs into separate folders for python2 and python3? If so would it be possible to share any stubs for boto/utils.py between multiple Python versions even if I have to split LockType into separate files? Any examples of this?

Other open questions, which I've worked around by using less specific stubs or removing stubs to get as far as I've gotten:

Should I add stubs for boto/provider.pyi because boto.utils depends on a boto.provider.Provider existing? Just want to confirm if "We generally shouldn't create a partial stub, because then users who use other functions from this module will get false positive errors that are hard to ignore." (emphasis mine) applies to only annotating classes or not. I had the stubs ready to go in a3e45b5 and I can add them back to this PR if you think it's best to include them.
How should I annotate the _Item.previous variable? Its type is _Item but mypy reports that error: Name '_Item' is not defined
How should I annotate LazyLoadMetadata.values(), LazyLoadMetadata.items(), and LRUCache.__contains__? I got errors that my annotations for them did not match the parent's annotations so I loosened them.
a. Should I use dict or Dict as the parent of these classes?
find_class returns a ClassType - is there a better annotation than Type[object] that I could use instead?
A lot of arguments in the ShellCommand class are straight passthroughs into functions in the subprocess module. To help ensure compatibility I borrowed the custom type annotations from the subprocess module, even though they are "private". Is there an alternative you would recommend?
JSONDecodeError's value in the code depends on whether or not simplejson is installed. Is there a better way to annotate its true type than what I have?

Any guidance is appreciated. Thanks!

JelleZijlstra · 2017-09-06T03:24:02Z

2017-09-05 13:45 GMT-07:00 Max Rozentsveyg <[email protected]>:

@JelleZijlstra <https://github.com/jellezijlstra> I'm almost done adding stubs here but I hit a roadblock. Any advice on the below errors? 1. third_party/2and3/boto/utils.pyi:162: error: Invalid type "boto.utils.Password.str" The Password class has a variable called str which mypy hiccups on. Should I remove the str variable from the stub or do something else? You can define an alias `_str = str` before the class and use that when

you mean the type instead of the class variable.

1. third_party/2and3/boto/utils.pyi:1: error: Cannot find module named '_thread' I'm trying to annotate with LockType from _thread (which exists in python2 and python3) but mypy isn't picking it up. Should I split my stubs into separate folders for python2 and python3? If so would it be possible to share any stubs for boto/utils.py between multiple Python versions even if I have to split LockType into separate files? Any examples of this? Looks like there is no stub for _thread in Python 2 (there is one in

Python 3). The best workaround is to make that stub exist. Perhaps you can move the existing _thread stub into stdlib/2and3 if the Python 2 and 3 APIs are sufficiently similar (I haven't checked). Otherwise you could add stdlib/2/_thread.pyi. If you don't want to go down that rabbit hole, it's OK to just use `Any` as the type annotation here, with a TODO to fix it once _thread exists in Python 2.

Other open questions, which I've worked around by using less specific stubs or removing stubs to get as far as I've gotten: 1. Should I add stubs for boto/provider.pyi because boto.utils depends on a boto.provider.Provider existing? Just want to confirm if "We generally shouldn't create a partial stub, because then users who use other *functions* from this module will get false positive errors that are hard to ignore." (emphasis mine) applies to only annotating classes or not. I had the stubs ready to go in a3e45b5 <a3e45b5> and I can add them back to this PR if you think it's best to include them. I didn't mean to exclude classes there. If we were to add a stub for

boto.provider, it should preferably be complete, for the same reason as before: otherwise people will get spurious type errors. The best solution would be to add the stubs for that module (though again, another workaround would be to replace the current boto.provider.Provider annotations with Any).

1. How should I annotate the _Item.previous variable? Its type is _Item but mypy reports that error: Name '_Item' is not defined I think that's because it's a nested class. Does `LRUCache._Item` work? 1. How should I annotate LazyLoadMetadata.values(), LazyLoadMetadata.items(), and LRUCache.__contains__? I got errors that my annotations for them did not match the parent's annotations so I loosened them. a. Should I use dict or Dict as the parent of these classes? The base class should preferably be `Dict[_KT, _VT]` where `_KT` and `_VT`

are either type variables or concrete types depending on the key and value types of these mappings. This way, methods like .get() automatically get the right type. Methods like .items() that return the type expected from the base class can then just be omitted.

1. find_class returns a ClassType - is there a better annotation than Type[object] that I could use instead? Perhaps `Type[Any]`, because otherwise all uses of the return value of

`find_class` will probably require an isinstance check.

1. A lot of arguments in the ShellCommand class are straight passthroughs into functions in the subprocess module. To help ensure compatibility I borrowed the custom type annotations from the subprocess module, even though they are "private". Is there an alternative you would recommend? That's fine. Our CI will fail if a change in subprocess breaks things here. 1. JSONDecodeError's value in the code depends on whether or not simplejson is installed. Is there a better way to annotate its true type than what I have? I think you can just allocate it as `JSONDecodeError: Type[Exception]`.

The current annotation as `ValueError` is incorrect, since it would mean that `JSONDecodeError` is an instance of `ValueError`.

Any guidance is appreciated. Thanks!

Thanks for your work here!

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1579 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3VaKm8DSfxGm41yhz65wTX72Iu570Hks5sfbLdgaJpZM4PFJlg> .

JelleZijlstra · 2017-09-06T03:24:48Z

Sorry for the mess in my emailed reply above; I hope it's still comprehensible.

mxr · 2017-09-12T12:05:45Z

@JelleZijlstra thanks for the feedback! I was able to work around the above errors using your advice. I err'd on the side of using Any with a TODO though.

I have some more questions able to annotate the _Item class internals with _KT and _VT. Using them to annotate key, value, and __init__() leads to errors such as

typeshed/third_party/2and3/boto/utils.pyi:143: error: Invalid type "typing._KT"
typeshed/third_party/2and3/boto/utils.pyi:144: error: Invalid type "typing._VT"
typeshed/third_party/2and3/boto/utils.pyi:145: error: Type variable '_KT' is bound by an outer class
typeshed/third_party/2and3/boto/utils.pyi:145: error: Type variable '_VT' is bound by an outer class

Any advice on the above? I left off the types for those 3 for now.

mxr · 2017-09-17T22:16:58Z

@JelleZijlstra friendly ping on the above. Thanks!

JelleZijlstra

Sorry for the delay! I just went over the code and over the implementation in https://github.com/boto/boto/blob/develop/boto/utils.py and noticed some small things.

JelleZijlstra · 2017-09-23T16:14:39Z

third_party/2and3/boto/utils.pyi

+def canonical_string(
+    method: str,
+    path: str,
+    headers: Dict[str, str],


Better to use Mapping since it doesn't actually need precisely a Dict.

JelleZijlstra · 2017-09-23T16:15:41Z

third_party/2and3/boto/utils.pyi

+    provider: Optional[_Provider] = ...,
+) -> str: ...
+def merge_meta(
+    headers: Dict[str, str],


Same here (and probably for most other Dict arguments)

JelleZijlstra · 2017-09-23T16:16:51Z

third_party/2and3/boto/utils.pyi

+        num_retries: int,
+        timeout: Optional[int] = ...,
+    ) -> None: ...
+    def get(self, key: _KT, default: Optional[Any] = ...) -> Any: ...


Just remove this since it's the same as the base class.

JelleZijlstra · 2017-09-23T16:18:36Z

third_party/2and3/boto/utils.pyi

+RFC1123 = ...  # type: str
+LOCALE_LOCK = ...  # type: _LockType
+
+def setlocale(name: Union[str, Tuple[str, str]]) -> None: ...


This should return a ContextManager[None].

Nice catch, I missed the yield. It looks like setlocale returns a string so would I want to use ContextManager[str]

typeshed/stdlib/2and3/locale.pyi

Line 84 in 7ecc979

def setlocale(category: int,

JelleZijlstra · 2017-09-23T18:03:39Z

third_party/2and3/boto/utils.pyi

+    file: Optional[IO] = ...,
+    username: Optional[str] = ...,
+    password: Optional[str] = ...,
+) -> IO: ...


Should be Optional. Also, not sure if this is an IO[bytes] or IO[str] from reading the code, but we should pick one.

I think str but not 100% sure...

JelleZijlstra · 2017-09-23T21:57:19Z

third_party/2and3/boto/utils.pyi

+    body: Optional[str] = ...,
+    html_body: Optional[Union[Sequence[str], str]] = ...,
+    to_string: Optional[str] = ...,
+    attachments: Optional[Iterable] = ...,


What kind of Iterable?

Each attachment is passed to email.message.Message.attach which is typed to be a Message

typeshed/stdlib/3/email/message.pyi

Line 31 in 7ecc979

def attach(self, payload: 'Message') -> None: ...

JelleZijlstra · 2017-09-23T21:57:49Z

third_party/2and3/boto/utils.pyi

+    attachments: Optional[Iterable] = ...,
+    append_instance_id: bool = ...,
+) -> None: ...
+def get_utf8_value(value: str) -> str: ...


Returns bytes

JelleZijlstra · 2017-09-23T21:59:39Z

third_party/2and3/boto/utils.pyi

+) -> str: ...
+def guess_mime_type(content: str, deftype: str) -> str: ...
+def compute_md5(
+    fp: IO,


This supports both kinds of IO, but it's good to be explicit and say IO[Any].

JelleZijlstra · 2017-09-23T22:00:33Z

third_party/2and3/boto/utils.pyi

+    size: Optional[int] = ...,
+    hash_algorithm: Any = ...,
+) -> Tuple[str, str, int]: ...
+def find_matching_headers(name: str, headers: Dict[str, str]) -> List[str]: ...


Use Mapping. In fact, it accepts any Iterable[str], but it's apparently not intended to.

JelleZijlstra · 2017-09-23T22:00:57Z

third_party/2and3/boto/utils.pyi

+    hash_algorithm: Any = ...,
+) -> Tuple[str, str, int]: ...
+def find_matching_headers(name: str, headers: Dict[str, str]) -> List[str]: ...
+def merge_headers_by_name(name: str, headers: Dict[str, str]) -> str: ...


Mapping, and the values can be Optional[str].

Perhaps I should change the value of all relevant headers variables to be Optional[str]?

I traced through the code and believe it is safe to do it for merge_headers_by_name, find_matching_headers, and canonical_string but not the others so I will go forward with that change

Sounds good

mxr · 2017-10-05T06:09:10Z

Thanks for reviewing and merging this @JelleZijlstra !

Add definitions for boto.utils.pythonize_name

e938f7f

JelleZijlstra requested changes Aug 29, 2017

View reviewed changes

Add stubs for provider.pyi and utils.pyi

a3e45b5

mxr changed the title ~~Add definitions for boto.utils.pythonize_name~~ Add definitions for boto/utils.py Sep 5, 2017

mxr force-pushed the patch-1 branch from 1e63d8a to 96b5c6b Compare September 5, 2017 20:40

fixes

a8ff43a

mxr force-pushed the patch-1 branch from 96b5c6b to a8ff43a Compare September 5, 2017 20:41

mxr force-pushed the patch-1 branch 4 times, most recently from 214222a to f2191f6 Compare September 12, 2017 02:51

Fix types

f138062

mxr force-pushed the patch-1 branch from f2191f6 to f138062 Compare September 12, 2017 11:26

Remove types

6c28d5c

Max Rozentsveyg added 2 commits September 12, 2017 05:21

More return values

3561d29

Add __str__ and fix __len__ return type

0fcbf5c

mxr force-pushed the patch-1 branch from 183188e to 0fcbf5c Compare September 12, 2017 16:44

JelleZijlstra requested changes Sep 23, 2017

View reviewed changes

Max Rozentsveyg added 5 commits September 26, 2017 18:50

Update types

908e839

Update

00105a0

Update

0baa5c1

Fix flake8 and remove _KT/_VT

8ba62da

Fix

6c480d8

JelleZijlstra approved these changes Oct 5, 2017

View reviewed changes

JelleZijlstra merged commit c68dcf1 into python:master Oct 5, 2017

mxr deleted the patch-1 branch October 5, 2017 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add definitions for boto/utils.py #1579

Add definitions for boto/utils.py #1579

mxr commented Aug 28, 2017 •

edited

Loading

JelleZijlstra Aug 29, 2017

mxr Aug 29, 2017

mxr commented Sep 5, 2017

JelleZijlstra commented Sep 6, 2017 via email

JelleZijlstra commented Sep 6, 2017

mxr commented Sep 12, 2017

mxr commented Sep 17, 2017

JelleZijlstra left a comment

JelleZijlstra Sep 23, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 27, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017 •

edited

Loading

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 27, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017

JelleZijlstra Sep 23, 2017

mxr Sep 26, 2017 •

edited

Loading

mxr Sep 26, 2017

JelleZijlstra Sep 26, 2017

mxr commented Oct 5, 2017

		@@ -0,0 +1,3 @@
		from typing import Text

		def pythonize_name(name: Text) -> Text: ...

Add definitions for boto/utils.py #1579

Add definitions for boto/utils.py #1579

Conversation

mxr commented Aug 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxr commented Sep 5, 2017

JelleZijlstra commented Sep 6, 2017 via email

JelleZijlstra commented Sep 6, 2017

mxr commented Sep 12, 2017

mxr commented Sep 17, 2017

JelleZijlstra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxr Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxr Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxr commented Oct 5, 2017

mxr commented Aug 28, 2017 •

edited

Loading

mxr Sep 26, 2017 •

edited

Loading

mxr Sep 26, 2017 •

edited

Loading