robotframework
robotframework copied to clipboard
robotframework
Conversion loses type information when using generic types
The supported conversions section of the documentation states:
Also types in in the typing module that map to the supported concrete types or ABCs (e.g. List) are supported. With generics also the subscription syntax (e.g. List[int]) works, but no validation is done for container contents.
It seems like a good enhancement to include these conversions as well, but the current code actually also contains a defect in this area. The converter_for method of TypeConverter strips the __origin__ information for all types but Union. As a result it is not possible to write any custom converters for generic types. Asuming you would typically need the nested types to do the proper conversions.
A pull request will follow to clarify the issue and demonstrate the suggested enhancement.
Interesting! I don't have time to look at this now, but I add this to RF 5.1 scope to not forget about it. Would be a nice enhancement to get into that release.
That would be great! I will try a Linux system with python 3.6 as well, to see what the unit test failures are about over there. I also gained some better insights in the LibDoc failures. A proposal will follow.
I've been busy with non-work stuff lately (mostly studying interpreters and compilers so it's been highly related) and haven't had time to look at this yet. I try to get beta out today but will look at this afterwords. This is in v5.1 milestone and won't be forgotten.
No problem, thanks for the update. I'll just consider it a compliment that you don't see it as a risk to integrate it that late in the process ;-) Although I also hope the tests I added are clear enough to show the exact impact.
The current status is that the Libdoc issues are solved without regression or changes. The Python 3.6 unit tests are fixed, but the 3.6 acceptance tests aren't. I haven't tried 3.7, but was able to run successful from Python 3.8 upward on Windows and Linux. The offer to disable the tests for Python 3.6 starts to look tempting. The issue originates from somewhere in the surrounding code, where the None-converter gets in the way. I lacked the time to deep dive into this part.
This has been now implemented based on the excellent PR #4434 by @JFoederer. Thanks a lot for your hard work!
I couldn't help but notice that the original line that started this issue, is now back in the code. The one that made a specific exception for Unions, which was previously the only type that supported nested types. Are we sure this won't bite us elsewhere?
That line is there, but converters nowadays get the used type, not the origin, as an argument. Possible origin is only used for finding the right converter. If you have examples where the current design causes problems, let me know!
No I don't, it all works fine. I was just looking over it again to get a better overview of how it all interacts including the Libdoc and custom converters part. I missed this point during the review and just wanted to make sure it wasn't reintroduced by accident.
Key change is the added
used_type = type_
before
if getattr(type_, '__origin__', None) and type_.__origin__ is not Union:
type_ = type_.__origin__
and that used_type is passed to converters instead of type_.
In RF 6.0 Libdoc doesn't handle parameterized types too well, but that is fixed in RF 6.1 (#4538)
This functionality unfortunately has a bug that when the used argument is an object, not a string, items are not converted (#4705). It will be fixed in RF 6.1.