-
Notifications
You must be signed in to change notification settings - Fork 2
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
dev: updated session for new schema #20
Conversation
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.
Looks good for the most part. A couple of small requests:
-
For the functions with arguments, we try to keep the docstring updated. I think pycharm highlights a warning if the function args don't match the descriptions in the docstrings. We settled on using NumPy's docstring format. There's a way in Pycharm to configure the format and auto-generate a template.
-
For the unit tests, it's ideal to compare the expected output with the actual output. In both the unit tests, there should be a line like
self.assetEqual(expected_output, actual_output)
. It's not foolproof , but it does give some indication of what should be expected in case someone makes updates to your code down the road.
closes #19