-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fixed read_precognition()
as per #135 and updated tests
#136
Conversation
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## main #136 +/- ##
=======================================
Coverage 98.67% 98.67%
=======================================
Files 41 41
Lines 1583 1590 +7
=======================================
+ Hits 1562 1569 +7
Misses 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Jack will reply later, but imo type checks should live in the cellify decorator which is called by the setter. |
Yeah, Jack slacked me about this as well. Makes sense, I'll take that out. |
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.
This looks good to me -- thanks for the PR!
As mentioned above, the setter methods for As such, this looks good -- when the |
Accomplished all of the changes mentioned in #135 . Two outstanding thoughts:
.ii
files are allowed, I didn't want to change the name of thehklfile
argument for fear of breaking backwards compatibility. That said, I know this function hasn't really seen a lot of daylight, so I'm happy to be overruled there.ValueError
for when acell
argument is provided but doesn't have length 6. Previously, the function checked for this length, but then just silently ignored an argument with the wrong length. I didn't add a test for this behavior, thinking that was overkill, but I could add that if desired.Also, as best I can tell, this is the first "warning" in
reciprocalspaceship
. If some mechanism other than the built-inwarnings
module is preferable, let me know.