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

Convert list type to tuple in frozen classes (Fixes #117) #129

Merged
merged 5 commits into from
Oct 4, 2018

Conversation

afsafzal
Copy link
Member

@afsafzal afsafzal commented Oct 3, 2018

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 50.18% when pulling 019a2d1 on frozen-list-to-tuple into ee91173 on master.

@coveralls
Copy link

coveralls commented Oct 3, 2018

Coverage Status

Coverage decreased (-0.2%) to 50.0% when pulling 5cf9726 on frozen-list-to-tuple into ee91173 on master.

@@ -20,21 +20,21 @@ class Mission(object):
configuration = attr.ib(type=Configuration)
environment = attr.ib(type=Environment)
initial_state = attr.ib(type=State)
commands = attr.ib(type=List[Command]) # FIXME
commands = attr.ib(type=Tuple[Command]) # FIXME
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use a converter to transform a list (or tuple) into a tuple.
See: http://www.attrs.org/en/stable/init.html#converters

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice. I didn't know that exists.

@@ -167,15 +167,15 @@ def generate_with_se(sut, initial, environment, config, mission):
sut = houston.ardu.ArduCopter(snapshot, config)

# mission description
cmds = [
cmds = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor point: It should be possible to pass a list to the constructor; the constructor should transform the list into a tuple.

@@ -20,21 +21,22 @@ class Mission(object):
configuration = attr.ib(type=Configuration)
environment = attr.ib(type=Environment)
initial_state = attr.ib(type=State)
commands = attr.ib(type=List[Command]) # FIXME
commands = attr.ib(type=Union[List[Command], Tuple[Command]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

The type property specifies the type of the values that are used by that attribute. To avoid misleading the static analysis, type should be Tuple[Command] in this case.

Copy link
Collaborator

@ChrisTimperley ChrisTimperley left a comment

Choose a reason for hiding this comment

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

Needs a small fix to the type used by two attributes.

@@ -72,13 +74,14 @@ class MissionOutcome(object):
a mission.
"""
passed = attr.ib(type=bool)
outcomes = attr.ib(type=List[CommandOutcome])
outcomes = attr.ib(type=Union[List[CommandOutcome], Tuple[CommandOutcome]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@afsafzal afsafzal merged commit db1f177 into master Oct 4, 2018
@afsafzal afsafzal deleted the frozen-list-to-tuple branch October 4, 2018 14:27
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.

None yet

3 participants