Trice/localization error weight plot

[trice9595]

This change is 

[ryan-summers]

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

scripts/localization_particle_probability_error_plot.py, line 16 at r1 (raw file):

class ScatterPlot():
    

Please kill off all your trailing whitespace (there's a few other spots below too)

---

scripts/localization_particle_probability_error_plot.py, line 17 at r1 (raw file):

class ScatterPlot():
    
    sub_point = Point()

Can you make sub_point a member of the class instead of a global variable?

---

scripts/localization_particle_probability_error_plot.py, line 20 at r1 (raw file):

    def euclidean_dist(self, particle):
        dist = ((self.sub_point.x - particle.x)**2 + (self.sub_point.y - particle.y)**2 + (self.sub_point.z - particle.z)**2)**0.5

I was actually doing this in my code as well. To ease readability, I would recommend using np.sqrt(particle.dot(particle)) or one of the other methods listed here - it's a bit more pythonic: https://stackoverflow.com/questions/9171158/how-do-you-get-the-magnitude-of-a-vector-in-numpy

Also, can you wrap this so it's under 80 characters?

---

scripts/localization_particle_probability_error_plot.py, line 27 at r1 (raw file):

Can you delete some of these extra newlines? Google python style guides typically recommend two new lines after each function.

---

scripts/localization_particle_probability_error_plot.py, line 50 at r1 (raw file):

         

    def __init__(self):

Can you move the init function to the first member of the class?

---

scripts/localization_particle_probability_error_plot.py, line 56 at r1 (raw file):

        # Subscribe to the topic
        self.sub = rospy.Subscriber("/simulator/cobalt/position", PointStamped, self.update_location)

It's typically bad practice to force root namespacing as it can cause issues if we ever run code in namespace'd regions. It should be fine to remove the leading forward slash.

---

scripts/localization_particle_probability_error_plot.py, line 65 at r1 (raw file):

    def __del__(self):
        self.sub.unregister()

Is this necessary? I'm fairly sure it will disappear when the node finishes anyways.

---

scripts/localization_particle_probability_error_plot.py, line 69 at r1 (raw file):

if __name__ == "__main__":
    global h

Variables within main are implicitly globally scoped in the script because main isn't technically a function, so this isn't necessary.

---

Comments from Reviewable

[skallaher]

Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

---

scripts/localization_particle_probability_error_plot.py, line 65 at r1 (raw file):

Previously, ryan-summers (Ryan Summers) wrote…

Is this necessary? I'm fairly sure it will disappear when the node finishes anyways.

@ryan-summers is correct here. This destructor is unnecessary.

---

Comments from Reviewable