Updated "Use __dict__ and ignore __slots__ on classes #169

[kylefawcett]

Fixes #168 and adds test case to prevent regression

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7c57ce1) 89.87% compared to head (5904a17) 90.24%.

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.
📢 Have feedback on the report? Share it here.

[pllim]

@Nodd , do you want to have a look at the added tests?

Thanks, all!

[kylefawcett]

@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.

[pllim]

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!

[kylefawcett]

Happy New Year! Anyone available to review this?

[pllim]

Hi, as people are just returning from their holiday breaks, I would give it another week or two. Thanks for your patience!

[pllim]

There are 2 jobs that failed. Can you please rebase and see if they go away? Thanks!

[pllim]

If rebase has nothing to do, please let me know, then I can restart those jobs directly. Thanks!

[bsipocz]

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.

[bsipocz]

Thank you @kylefawcett!