Fix validating_code_block typecheck errors #1949
Conversation
✅ Deploy Preview for nifty-bassi-e26446 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
mmorel-35
force-pushed
the
fix-typecheck
branch
7 times, most recently
from
acaa847
to
e3c3b1b
Compare
mmorel-35
changed the title
Signed-off-by: Matthieu MOREL <[email protected]>
mmorel-35
force-pushed
the
fix-typecheck
branch
from
e3c3b1b
to
18fee18
Compare
| @@ -26,8 +26,7 @@ class ValidatingCodeBlock(CodeBlock): | |||
| required_arguments = CodeBlock.required_arguments | |||
| optional_arguments = CodeBlock.optional_arguments | |||
| final_argument_whitespace = CodeBlock.final_argument_whitespace | |||
| option_spec = {"type-name": directives.unchanged} | |||
| option_spec.update(CodeBlock.option_spec) | |||
| CodeBlock.option_spec.update({'type-name': directives.unchanged}) | |||
There was a problem hiding this comment.
im not so familiar with validating code block - mostly we dont use it as it makes docs build a lot slower
my concern with this change is that its setting CodeBlock.option_spec for any subsequent usage
with python its generally an anti-pattern to use any mutable vars on the class - for this reason - other code can change/redefine it and its not obvious on the class (or for any other instantiations). Mutable vars are generally defined post-instantiation - either in __init__ or by returning a mutable var from a cached @property
in this case im surprised the Sphinx code allows it, but either way i think this is changing something it shouldnt
looking at the typing errors
envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/ext/validating_code_block.py:29: error: Cannot override instance variable (previously declared on base class "CodeBlock") with class variable
envoy.docs.sphinx_runner/envoy/docs/sphinx_runner/ext/validating_code_block.py:30: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[str, Callable[[str], Any]]"; expected "SupportsKeysAndGetItem[str, Callable[[Optional[str]], str]]"in not sure on cause or fix tbh
in particular im trying to understand why this is flaking on some PRs and not others
the underlying issue appears to be related to this python/typeshed#11550
There was a problem hiding this comment.
... i see - its the mypy update PR that is failing here
There was a problem hiding this comment.
also relevant sphinx-doc/sphinx#12067
There was a problem hiding this comment.
iiuc this PR would still fail the newer mypy
There was a problem hiding this comment.
just realizing that all prs are failing this which is strange
trying to understand what changed - it kinda shouldnt have started without some code change
either way we need to address it asap
There was a problem hiding this comment.
checking again and we dont actually pin versions (kinda by design, but perhaps we should do so for external deps)
some are set by pants/mypy setup - not sure how they can be pinned
@phlax ,
I'm not extremely familiar with python so I prefer to check with you
The test_extensions.py / test_ext_validating_code_block_vbc_constructor is still passing.
I believe that fixes the typecheck error or am I missing something ?