Conversation
eepeterson
left a comment
There was a problem hiding this comment.
Nice work!! first round of comments. Generally we may want to factor out some of the plotting functionality and scripting functionality, but first let's work on the class itself.
| import matplotlib.pyplot as plt | ||
|
|
||
| class Shot: | ||
| """Shot class to store data from a shot.""" |
There was a problem hiding this comment.
You need a class docstring that documents Parameters and Attributes - take a look at PR #2 to see an example that has been through review
| self.load_csv() | ||
|
|
||
|
|
||
| def load_csv(self): |
There was a problem hiding this comment.
| def load_csv(self): | |
| @classmethod | |
| def from_csv(self, path=None): |
This function feels a bit awkward, because the user has no control over the load_csv method (i.e. no arguments to pass). If this is only used in the constructor then it's probably not worth having as a method on it's own. The way from_file methods typically work is as class methods to give the user the option of specifying a path.
|
|
||
| return fig, axs | ||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
if __name__ == "__main__": shouldn't really be used in module files I don't think. Typically these aren't intended to be run by the command line, but scripts that should be run like this go in a separate place we can look at.
|
Can you rebase on main so that tests run? |
RemDelaporteMathurin
left a comment
There was a problem hiding this comment.
Can we add some tests to this feature?
Shot.py introduces a "shot object" that we load data into
Helper functions allow plotting in matplotlib style
Example of how it can be used are in shot.py main method