search

eclipse-cyclonedds / cyclonedds-python

Other
54 stars 44 forks source link

Reserved Python keywords support (Issue 105) #163

Closed javiersorianoruiz closed 1 year ago

javiersorianoruiz commented 1 year ago

We (@Bit-Seq, @jordialexalar and me) have implemented a solution for the issue 105.

https://github.com/eclipse-cyclonedds/cyclonedds-python/issues/105

With this change, now, the IDL file can contain reserved Python keywords and when this IDL file is translated into Python the IDLC Python plugin has been modified to add a prefix "_" to the found reserved keywords.

We have had into account all the posible declaration using IDL: structs, modules, enums, typedefs...

We have tested broadly this change. The developed and executed tests can be review in detail in this git repository:

https://github.com/javiersorianoruiz/cyclonedds-python-checkReservedKeyword

These are not the formal tests that maybe are required by cyclonedds-python project, but they have allowed us to verify that the implementation is right and have no colateral issues.

Maybe, It would be needed to develop another test batery based on pytest and integrated into cyclonedds-python project, following your testing architecteture, What do you think?, Is it needed?

Bit-Seq commented 1 year ago

Another detail not mentioned above. True and False keywords are not supported, due to they are also keywords in the IDL.

The IDLC compiler returns a syntax error even before trying to convert the IDL file to Python, so IDLC does not support these words in an IDL file for any supported language.

Anyway, True and False are keywords in almost all languages, and it is rare a lot to try to define an object with those identifiers.

javiersorianoruiz commented 1 year ago

Hello @thijsmie

We have updated the code with your suggested changes. So you can advance in the pull request and merge process.

Everything is working properly in our testing: https://github.com/javiersorianoruiz/cyclonedds-python-checkReservedKeyword

We have pending:

About "we can also resolve types dynamically from the network using the XTInterpreter", How do we have to do this? How do we have to write the IDL file for the method XTInterpreter._make_complete_struct or XTInterpreter._make_complete_union to be executed during the translation from IDL to Python?

If you give us more information about how to reproduce this conversion, we can try to implement the change ourselves as we have done with the rest of cases.

We have added also the keywords you ask for them, so, consider closing Issue #161 if the pull request is ok for you.