refactor StellarGradebook code #11
Conversation
…Client classes - untested
|
Looks like you have some pep-8 stuff to deal with |
pylmod/gradebook.py
Outdated
There was a problem hiding this comment.
Please add an actual docstring. Preferably the google style class doc string, but a simple description is fine too
|
https://github.com/mitodl/PyLmod/blob/feature/refactor-stellargradebook/pylmod/stellargradebook.py should no longer be here, and we should see it being removed from the diff. It has the added benefit that we can see better which pieces got moved where. |
|
In my enthusiasm to get this committed I left numerous pylint and pep8 errors. I'm working on those now. I really only wanted to verify that code is split up properly for this PR. |
|
No problem, it is just hard to tell without stellargradebook.py being deleted if you could commit that real quick I can ignore the style problems |
|
I'd like eyes on this line: https://github.com/mitodl/PyLmod/pull/11/files#diff-a2ca8308d9281e045010f13ce409eb81R76 |
|
Sure, so nothing in base.py uses that property and that initialization should move to the gradebook class |
… is still flawed though. I'm committing these changes so we can consider whether we need another class that inherits from both Membership and Gradebook.
…returned in __init__.py which I can't figure out
pylmod/__init__.py
Outdated
There was a problem hiding this comment.
You shouldn't have base here
|
I'm happy to merge after those last few comments are addressed. What a long strange trip... 🐍 |
pylmod/base.py
Outdated
There was a problem hiding this comment.
it would be nice to at least correct the class name here.
There was a problem hiding this comment.
Corrected the class name and the 'sg' variable as well. I also removed "stellargradebook" and "stellar" from all files where it was possible.
refactor StellarGradebook code
2 participants
Refactor the StellarGradebook code into Client, Base, Membership, and Gradebook classes. The intent of this PR is simply to separate the code into separate classes, nothing more. If this PR is reviewed as something more comprehensive, I'd prefer to resubmit the PR so that it specifically identifies the broader goal.
Addresses #7, #8, #9, and #10.
This PR is ready for review. @pdpinch @carsongee