Remove STUserDict
#191
Remove STUserDict
#191
Conversation
|
Leaving this as a draft until all the regression tests are functional again. This will need to be carefully tested. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #191 +/- ##
==========================================
- Coverage 97.93% 97.84% -0.10%
==========================================
Files 9 8 -1
Lines 1258 1204 -54
==========================================
- Hits 1232 1178 -54
Misses 26 26
☔ View full report in Codecov by Sentry. |
| def find_mod_objs_patched(*args, **kwargs): | ||
| from sphinx_automodapi.utils import find_mod_objs | ||
|
|
||
| return find_mod_objs(args[0], onlylocals=True) | ||
|
|
||
|
|
||
| def patch_automodapi(app): | ||
| """Monkey-patch the automodapi extension to exclude imported members""" | ||
| from sphinx_automodapi import automodsumm | ||
|
|
||
| automodsumm.find_mod_objs = find_mod_objs_patched | ||
|
|
||
|
|
There was a problem hiding this comment.
Monkey patches like this are generally a bad idea. The whole reason for the need of this patch was poor construction of stuserdict.py module to begin with. #156 (comment) details a couple of different ways of properly resolving this without monkey patching. However, all of those options are moot because this PR removes stuserdict.py, which side steps the issue completely.
WilliamJamieson
force-pushed
the
refactor/remove_stuserdict
branch
from
0694c53 to
3962d7c
Compare
|
The regression tests passed: https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-devdeps/298/ so this is ready. |
WilliamJamieson
force-pushed
the
refactor/remove_stuserdict
branch
2 times, most recently
from
85b1346 to
0bf3808
Compare
These are important to demonstrate the copy behavior does not change when we remove STUserDict
We imply these stnode based objects should be self copyable, so this makes them self copyable.
It is no longer being used.
WilliamJamieson
force-pushed
the
refactor/remove_stuserdict
branch
from
0bf3808 to
0dac8ab
Compare
STUserDictis essentially a copy of the Python standard library'scollections.UserDictsource code with the change of name and the change of thedata(UserDict) variable to_data(STUserDict). This copy was a little while ago and is no longer "in-sync" with the standard library's version.It turns out that this approach is a bit "overkill" for what
DNode(what usesSTUserDict) actually needs. Instead it just needs to be an abstract mutable mapping (abc.MutableMapping).This PR switches
DNodefrom usingSTUserDicttoabc.MutableMappingwhich solves this issue. It also removesSTUserDictbecause it is no-longer necessary.Checklist
CHANGES.rstunder the corresponding subsection