Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upIssue #184 and #239 Added types and defaults to function help text. #251
Conversation
|
I've just ran pylint and it looks like I got some of the indentation wrong, so I'll fix that tomorrow. Also running pylint with a py2 interpreter doesn't like some of the py3 only tests, so I'll add pylint disable comments to those. |
|
I'd handled the tests using type hints by putting them in a string and conditionally calling exec on them if the version was >3.5. I've now changed this so that they are in a conditionally imported module, following the approach taken with fire/test_components_py3.py |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| # Lint as: python3 |
MichaelCG8
Apr 27, 2020
Author
Contributor
I've copied this directive from fire/test_components_py3.py, but haven't come across it before and can't find mention of it in pylint's documentation. If there is a more specific option such as # Lint as: >python3.5 then that would be better.
dbieber
Apr 27, 2020
Member
This is a Google-internal directive.
We'll modify our codebase syncing so its removed from the public codebase, but for now let's keep it so things work internally too.
|
I expect I'll get to the rest of the review this Friday. |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| # Lint as: python3 |
dbieber
Apr 27, 2020
Member
This is a Google-internal directive.
We'll modify our codebase syncing so its removed from the public codebase, but for now let's keep it so things work internally too.
|
Looks good! We'll make some small changes as part of merging, but no need to take additional action on your part. One point for discussion: You truncate types and defaults with |
|
Good suggestion. I started with I see that custom_descriptions.GetStringTypeSummary uses formatting.EllipsisTruncate, I've applied that to the code and it tidies things up a lot. |
|
Is there any update on when is this going to be merged? |
|
We ran into an error when merging initially. The long_type test fails internally:
Thanks for your patience. |
|
Hi David, thanks for looking at this! |
|
It's Python 3.6.7 that it's failing on, but I think we may be using a nonstandard
I made a few small changes but nothing substantial. |
|
Merged with 79d85df |
No description provided.