Skip to content

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

Merged
aaltat merged 8 commits intorobotframework:masterfrom
aaltat:fix_none_hints
Jun 1, 2020
Merged

None and perhaps bool type should not return type hints when used as defaults. #58
aaltat merged 8 commits intorobotframework:masterfrom
aaltat:fix_none_hints

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