Skip to content

None and perhaps bool type should not return type hints when used as defaults. #58

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

Merged
merged 8 commits into from
Jun 1, 2020

Conversation

aaltat
Copy link
Contributor

@aaltat aaltat commented May 24, 2020

Fixes #60

@aaltat aaltat force-pushed the fix_none_hints branch 2 times, most recently from 2cddd0d to c54080d Compare May 24, 2020 21:37
@aaltat aaltat requested a review from pekkaklarck May 24, 2020 21:46
@pekkaklarck
Copy link
Member

Code changes look good but we can still discuss is this the best solution to fix the problem on showing unnecessary NoneType (and to some extend bool) in Libdoc HTML outputs and specs. Alternative solutions:

  • Handle all this on Libdoc side.

  • Add bool and perhaps also NoneType from defaults only with RF 3.1. With RF 3.2 there's no need to do that since it gets the real default values and can do automatic conversion based on them.

@aaltat
Copy link
Contributor Author

aaltat commented May 29, 2020

I think doing something in libdoc side is needed in any case, because PLC is not the only implementation of the dynamic library API (at least I hope so, although it might be the only one currently using the new functionality of the API.)

But I agree that we should change PLC implementation to return defaults differently for 3.1 and 3.2+. From the SeleniumLibrary keywords docs will be generated by using RF 3.2, so from that point of view PLC decisions on RF 3.1 are not significant.

  1. For RF 3.2 bool and NoneType hints should not be returned by PLC.
  2. PLC should return bool hints with 3.1.
  3. PLC could also return NoneType hint in RF 3.1, at least I can not see any real harm on it.

@pekkaklarck
Copy link
Member

The more I think about this, the better the solution to add both bool and NoneType based on default values to types but only with RF 3.1 feels.

I'm not sure are changes needed on Libdoc side now that dynamic libs can return real default values. It could be argued that if you explicitly return a type from get_keyword_types it should be shown in docs as well. Same as if you explicitly use something like arg: NoneType in a static keyword. Libdoc should probably format argument names, types and default values differently, though, but that's not directly related to this issue.

@aaltat aaltat merged commit edd411d into robotframework:master Jun 1, 2020
@aaltat aaltat deleted the fix_none_hints branch June 1, 2020 20:55
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.

Fix typing hints for None and bool types
2 participants