Add string representation for the procedures classes

[x-jesse]

This change is 

[wendi-yu]

This looks like a really good start, thanks for doing this! There are a few small fixes to address, but other than that this is super helpful for debugging and general QOL :)

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @x-jesse)

---

topside/procedures/procedure.py, line 78 at r1 (raw file):

    def __str__(self):
        return "Step ID: " + self.step_id + "\nAction: {" + self.action.__str__() + "}\nConditions: " + str(self.conditions)

Python has format strings which are pretty useful for formatting more complex stuff like this, they look something like

name = Joe
string = f"Bob says hi to {name}"

and then you'd end up with "Bob says hi to Joe". Might be easier to use here than manual string concatenation :) I think triple quotes also allow for you to manually put in newlines (instead of using \n, so using these two might make the formatting more easily readable just from the code.

I like that we're using the string representation on the action here though, feels very elegant :) in fact I don't think you need to call the __str__() method at all, it'll just do it automatically for you here.

---

topside/procedures/procedure.py, line 78 at r1 (raw file):

    def __str__(self):
        return "Step ID: " + self.step_id + "\nAction: {" + self.action.__str__() + "}\nConditions: " + str(self.conditions)

If you want to, we could also expand the scope of the PR to add string representations to the classes in conditions.py as well. Right now, the Conditions: part looks like this when printed, since Condition objects don't have a string representation yet:

Conditions: {<topside.procedures.conditions.Immediate object at 0x000001DB4DB90B48>: Transition(procedure='p1', step='s3')}

dicts containing objects also don't print super nicely (the __str()__ method doesn't automatically get called on contained objects), so it might be nicer to manually iterate through the conditions dict here and add stuff to a string.

---

topside/procedures/procedure.py, line 103 at r1 (raw file):

    def __str__(self):
        return "Procedure ID: " + self.procedure_id + "\nSteps: " + self.steps

I'm getting an error when I try to print a procedure:

  File "c:\users\disco\onedrive\documents\uni stuff\rocketry\operations-simulator\topside\procedures\procedure.py", line 103, in __str__
    return "Procedure ID: " + self.procedure_id + "\nSteps: " + self.steps
TypeError: can only concatenate str (not "dict") to str

I think you need at least a str(self.steps) here, since self.steps is a dict. Might be worth throwing in a unit test that tries to string and print all of these types, just to make sure that everything works. The same thing with dicts not printing nicely applies here though, might be nice to do something custom with the steps dict.

[wendi-yu]

Looks good overall! There are still a few comments from last time that need to be addressed, and it would be really great if you could add unit tests that print an instance of each of these classes as well, just to make sure we're not erroring.
Again, happy to give a walkthrough of adding unit tests if you'd like :)

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @jacobdeery, @wendi-yu, and @x-jesse)

---

requirements.txt, line 6 at r2 (raw file):

matplotlib==3.2.0
networkx==2.4
numpy

Hey just remember to restore this please, we removed the pin initially as a workaround to be able to install numpy on your system. We probably don't want this on master :)

---

topside/procedures/procedure.py, line 78 at r1 (raw file):

Previously, wendi-yu (Wendi Yu) wrote…

If you want to, we could also expand the scope of the PR to add string representations to the classes in conditions.py as well. Right now, the Conditions: part looks like this when printed, since Condition objects don't have a string representation yet:

Conditions: {<topside.procedures.conditions.Immediate object at 0x000001DB4DB90B48>: Transition(procedure='p1', step='s3')}

dicts containing objects also don't print super nicely (the __str()__ method doesn't automatically get called on contained objects), so it might be nicer to manually iterate through the conditions dict here and add stuff to a string.

Can move this to another PR like we discussed

---

topside/procedures/procedure.py, line 103 at r1 (raw file):

Previously, wendi-yu (Wendi Yu) wrote…

I'm getting an error when I try to print a procedure:

  File "c:\users\disco\onedrive\documents\uni stuff\rocketry\operations-simulator\topside\procedures\procedure.py", line 103, in __str__
    return "Procedure ID: " + self.procedure_id + "\nSteps: " + self.steps
TypeError: can only concatenate str (not "dict") to str

I think you need at least a str(self.steps) here, since self.steps is a dict. Might be worth throwing in a unit test that tries to string and print all of these types, just to make sure that everything works. The same thing with dicts not printing nicely applies here though, might be nice to do something custom with the steps dict.

There's still the issue with self.steps here, you can see it yourself if you add a unit test that just calls print on an instance of each data structure. Running pytest will then catch these errors for you. You probably want to iterate through self.steps and construct a string representation, maybe something like

step_string = ''
for key, value in self.steps.items():
    step_string = step_string + f"\n {key}: {value}"
return f"""Procedure ID: {self.procedure_id}    Steps: {self.steps}

Feel free to message if you'd like help or just a walkthrough of setting up unit tests!

[x-jesse]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @jacobdeery, @wendi-yu, and @x-jesse)

---

requirements.txt, line 6 at r2 (raw file):

Previously, wendi-yu (Wendi Yu) wrote…

Hey just remember to restore this please, we removed the pin initially as a workaround to be able to install numpy on your system. We probably don't want this on master :)

Sounds good!

[wendi-yu]

Looks good, just the unit test to consider now. I think you also might have merged incompletely with master, I can walk you through a rebase to update your PR and get rid of the merge conflicts

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @jacobdeery, @wendi-yu, and @x-jesse)

---

topside/procedures/procedure.py, line 103 at r1 (raw file):

Previously, wendi-yu (Wendi Yu) wrote…

There's still the issue with self.steps here, you can see it yourself if you add a unit test that just calls print on an instance of each data structure. Running pytest will then catch these errors for you. You probably want to iterate through self.steps and construct a string representation, maybe something like

step_string = ''
for key, value in self.steps.items():
    step_string = step_string + f"\n {key}: {value}"
return f"""Procedure ID: {self.procedure_id}    Steps: {self.steps}

Feel free to message if you'd like help or just a walkthrough of setting up unit tests!

Cool, this is no longer erroring. It would be good to set up a unit test for these just to make sure that the __str__() function is working as intended though

[codecov]

Codecov Report

Merging #54 (bdb5a96) into master (afe80ed) will decrease coverage by 0.58%.
The diff coverage is 54.16%.

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
- Coverage   91.84%   91.26%   -0.59%     
==========================================
  Files          25       25              
  Lines        2047     2071      +24     
  Branches      380      387       +7     
==========================================
+ Hits         1880     1890      +10     
- Misses        123      137      +14     
  Partials       44       44              

Impacted Files	Coverage Δ
topside/procedures/procedure.py	78.46% <54.16%> (-8.34%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afe80ed...bdb5a96. Read the comment docs.