Updated "Use __dict__ and ignore __slots__ on classes #169#181
Updated "Use __dict__ and ignore __slots__ on classes #169#181bsipocz merged 8 commits intoastropy:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #181 +/- ##
==========================================
+ Coverage 89.87% 90.24% +0.36%
==========================================
Files 27 28 +1
Lines 1146 1179 +33
==========================================
+ Hits 1030 1064 +34
+ Misses 116 115 -1 ☔ View full report in Codecov by Sentry. |
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
|
@Nodd , do you want to have a look at the added tests? Thanks, all! |
|
@pllim Circling back around on this. I just cleaned up the slots example module to be more precisely consistent with python nomenclature and added a basic test for the example to meet the coverage thresholds. The other existing examples don't have tests so I'm not sure if you want to set a precedent. I can just pop off the last commit if you don't want tests for the slot example module. |
|
More tests is good, so no need to undo it. Thanks! I tag some people for review because I am not familiar with slots or automodapi. But if no one responds, I can take a stab at reviewing after the holidays. Happy holidays! |
|
Happy New Year! Anyone available to review this? |
|
Hi, as people are just returning from their holiday breaks, I would give it another week or two. Thanks for your patience! |
|
There are 2 jobs that failed. Can you please rebase and see if they go away? Thanks! |
|
If rebase has nothing to do, please let me know, then I can restart those jobs directly. Thanks! |
There was a problem hiding this comment.
I'm not familiar with slots either, but dug up the history of this part of the code. It was added many years ago in #53, along with test cases, which still pass after this PR.
So I would say go ahead and merge this if CI is full green.
|
Thank you @kylefawcett! |
Fixes #168 and adds test case to prevent regression