Skip to content
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

Parser classes and type hints #273

Open
klauer opened this issue Nov 10, 2021 · 0 comments
Open

Parser classes and type hints #273

klauer opened this issue Nov 10, 2021 · 0 comments

Comments

@klauer
Copy link
Contributor

klauer commented Nov 10, 2021

Parser classes background

So there are some unfortunate things in the way that the parser breaks out TwincatItem classes. Here's the background on why the parser items are as they are:

  • I wanted easy mappings of TwinCAT <xml-tag> to Python class, where methods and properties could then be context-specific (e.g., give me the source code of the GVL)
  • I wanted things like gvl.Declaration or plc.TcSmItem.PlcProject.Mappings to be a thing you could do, since I thought __getitem__-based plc["TcSmItem"]["PlcProject"]["Mappings"] was verbose and ugly
  • I was unaware of which XML items would be able to have more than one entry in a given scope. (In retrospect, TwinCAT has xsds that provide this information)
  • This led to the current (ugly) structure of plc.TcSmItem[0].PlcProject[0].Mappings[...], along with all of the code that grabs the only element from an assumed single-element list

What about better type hints?

Now, type hints weren't really a thing when pytmc was getting on its feet, at least with the versions we used and whatnot.
So type hints in TwincatItem subclasses are unfortunately really bad, given how the classes are structured:

class EnumInfo(_TmcItem):
    # Some have type hints like this now
    Comment: list  
    # What we really want to say to describe the structure as-is
    Comment: Optional[typing.List[Comment]]  
    # With a refactor to de-listify single items, this would be ideal
    Comment: Optional[Comment]  

The latter two appear to be circular references rather than one to a corresponding Comment class.

Where do we go from here? Though I'd really like to see this improved, I guess we just leave it as-is until a real refactor could be justified - unless anyone has a better idea.

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

No branches or pull requests

1 participant