Skip to content

Conversation

@wdednam
Copy link
Contributor

@wdednam wdednam commented Nov 29, 2024

Thought this one would be more straightforward, but it seems that the C++-wrapped findObject method does not expose more attributes to the pyminsky module than those in the item class, and, as a result, one can't access subattributes of specific item types directly from the object returned by findObject, e.g., name, valueId, etc. Thus, I have resorted to iterating minsky.model.items to expose the attribute required for the test (valueId).


This change is Reviewable

Copy link
Contributor Author

@wdednam wdednam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @wdednam)

@highperformancecoder
Copy link
Owner

Thought this one would be more straightforward, but it seems that the C++-wrapped findObject method does not expose more attributes to the pyminsky module than those in the item class, and, as a result, one can't access subattributes of specific item types directly from the object returned by findObject, e.g., name, valueId, etc. Thus, I have resorted to iterating minsky.model.items to expose the attribute required for the test (valueId).

You're right - polymorphic objects are not currently supported on methods returning references. They should be of course, just like smart pointers support polymorphic behaviour of classes that inherit from PolyRESTProcessBase.

Also functions that return smart pointers should be supported, but don't work at all for some reason.

I'm going to add tickets to the classdesc project to do these, but it might be a while before getting to them.

value_id = None
for i in range(len(minsky.model.items)):
item = minsky.model.items[i]
if item.x() == integralVariable.x() and item.y() == integralVariable.y():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Th Item::id() method is a more direct way of determining items match.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also - in this case, the items list will have exactly two items - an IntOp and a Variable::integral. So we could just examine Item::classType()

Copy link
Owner

@highperformancecoder highperformancecoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @wdednam)

@highperformancecoder highperformancecoder merged commit b5759bc into highperformancecoder:master Nov 29, 2024
4 of 5 checks passed
@wdednam wdednam deleted the pythoniseRegTests branch December 13, 2024 08:14
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.

2 participants