-
Notifications
You must be signed in to change notification settings - Fork 3
rebased to the recent master #15
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
base: master
Are you sure you want to change the base?
Conversation
members = {} | ||
for s, stype in six.iteritems(members_types): | ||
schema_instance = RosSchema() | ||
for s, stype in members_types.iteritems(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iteritems() is not py2/py3 compatible. please use methods that work in both.
members_types = _get_rosmsg_fields_as_dict(ros_msg_class) | ||
members = {} | ||
for s, stype in six.iteritems(members_types): | ||
schema_instance = RosSchema() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you mean that instead of creating a child class (CustomMsgSchema) of RosSchema, and taking and instance of that class,
it is better to instantiate a RosSchema and usedirectly that instance ?
How about the second time we need that Schema, we have to create it again it seems ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to solve your source as much as possible without modifying it if possible. So I did not try various methods. Sorry and thank you of your efforts.
ros_schema_inst = RosNested(create(stype)) # we need to nest the next (Ros)Schema | ||
|
||
members.setdefault(s, ros_schema_inst) | ||
schema_instance.declared_fields[s] = ros_schema_inst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems very hacky to me.
If marshmallow is updated and decides to change the internals of RosSchema, nothing will work and it will be hard to find this place again. Why not initializing the instance after we have gathered all members ?
my branch is rebased on your the recent master
I hope to pass on travis
Thanks!