From 3973e897d18aa0b646e7f2bf9d90aba87b9d3a98 Mon Sep 17 00:00:00 2001 From: Viktor Haag Date: Thu, 13 Jun 2019 14:01:57 -0400 Subject: [PATCH 1/5] Explicitly treat ObjectDescription._doc_field_type_map as an instance variable - Aims to address [issue #6478](https://github.com/sphinx-doc/sphinx/issues/6478) - Left ObjectDescription._doc_field_type_map declaration alone and in place in case there are other factors at play that require it declared on the class. - In ObjectDescription constructor, explicitly set _doc_field_type_map's value as an instance variable. - This presumably needs to be built with every instantiation of an ObjectDescription or inheriting class, so we're not attempting to "define it once on a class and then use it"... that approach seems much harder to reason about and get correctly done (as was maybe demonstrated by the refactoring that lead to this problem in the linked-to issue). - Runs through the test suite clean locally except for an unrelated single test failure on an image_converter test, which also failed on the base branch before my changes here. --- sphinx/directives/__init__.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/sphinx/directives/__init__.py b/sphinx/directives/__init__.py index 40f838c483d..68a27090f86 100644 --- a/sphinx/directives/__init__.py +++ b/sphinx/directives/__init__.py @@ -70,18 +70,23 @@ class ObjectDescription(SphinxDirective): # Warning: this might be removed in future version. Don't touch this from extensions. _doc_field_type_map = {} # type: Dict[str, Tuple[Field, bool]] + def _process_type_map(self, typelist): + typemap = {} + for field in typelist: + for name in field.names: + typemap[name] = (field, False) + if field.is_typed: + typed_field = cast(TypedField, field) + for name in typed_field.typenames: + typemap[name] = (field, True) + return typemap + + def __init__(self, *args, **kwargs): + SphinxDirective.__init__(self, *args, **kwargs) + self._doc_field_type_map = self._process_type_map(self.doc_field_types) + def get_field_type_map(self): # type: () -> Dict[str, Tuple[Field, bool]] - if self._doc_field_type_map == {}: - for field in self.doc_field_types: - for name in field.names: - self._doc_field_type_map[name] = (field, False) - - if field.is_typed: - typed_field = cast(TypedField, field) - for name in typed_field.typenames: - self._doc_field_type_map[name] = (field, True) - return self._doc_field_type_map def get_signatures(self): From 085ae2e08187acc5dfdff7e79364e5962afc121c Mon Sep 17 00:00:00 2001 From: Viktor Haag Date: Thu, 13 Jun 2019 14:29:21 -0400 Subject: [PATCH 2/5] type declaration for new internal method --- sphinx/directives/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/directives/__init__.py b/sphinx/directives/__init__.py index 68a27090f86..72ab221d288 100644 --- a/sphinx/directives/__init__.py +++ b/sphinx/directives/__init__.py @@ -71,6 +71,7 @@ class ObjectDescription(SphinxDirective): _doc_field_type_map = {} # type: Dict[str, Tuple[Field, bool]] def _process_type_map(self, typelist): + # type: () -> Dict[str, Tuple[Field, bool]] typemap = {} for field in typelist: for name in field.names: From d19dd1c8dec8cc80229e8314cdbc37650b366eaf Mon Sep 17 00:00:00 2001 From: Viktor Haag Date: Fri, 14 Jun 2019 08:02:24 -0400 Subject: [PATCH 3/5] mypy type annotations for methods added - type annotation for _process_type_map was incorrect; needed to document the positional argument for the method - __init__ method for ObjectDescription missing type annotation, so added --- sphinx/directives/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sphinx/directives/__init__.py b/sphinx/directives/__init__.py index 72ab221d288..3eb3866f4db 100644 --- a/sphinx/directives/__init__.py +++ b/sphinx/directives/__init__.py @@ -71,7 +71,7 @@ class ObjectDescription(SphinxDirective): _doc_field_type_map = {} # type: Dict[str, Tuple[Field, bool]] def _process_type_map(self, typelist): - # type: () -> Dict[str, Tuple[Field, bool]] + # type: (List[Field]) -> Dict[str, Tuple[Field, bool]] typemap = {} for field in typelist: for name in field.names: @@ -83,6 +83,7 @@ def _process_type_map(self, typelist): return typemap def __init__(self, *args, **kwargs): + # type: (*Any, **Any) -> None SphinxDirective.__init__(self, *args, **kwargs) self._doc_field_type_map = self._process_type_map(self.doc_field_types) From 6a054355d92628fbfa4670f097b5ce20db6fdc2e Mon Sep 17 00:00:00 2001 From: Viktor Haag Date: Tue, 18 Jun 2019 09:22:25 -0400 Subject: [PATCH 4/5] Simplify to ensure we're lazily setting the instance field-type-map variable - We can more simply have ObjectDescription.get_field_type_map() make sure that it's going to lazily set the value of an instance variable, not the class variable, when it populates the field type map. --- sphinx/directives/__init__.py | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/sphinx/directives/__init__.py b/sphinx/directives/__init__.py index 3eb3866f4db..8a17fc0a520 100644 --- a/sphinx/directives/__init__.py +++ b/sphinx/directives/__init__.py @@ -70,25 +70,17 @@ class ObjectDescription(SphinxDirective): # Warning: this might be removed in future version. Don't touch this from extensions. _doc_field_type_map = {} # type: Dict[str, Tuple[Field, bool]] - def _process_type_map(self, typelist): - # type: (List[Field]) -> Dict[str, Tuple[Field, bool]] - typemap = {} - for field in typelist: - for name in field.names: - typemap[name] = (field, False) - if field.is_typed: - typed_field = cast(TypedField, field) - for name in typed_field.typenames: - typemap[name] = (field, True) - return typemap - - def __init__(self, *args, **kwargs): - # type: (*Any, **Any) -> None - SphinxDirective.__init__(self, *args, **kwargs) - self._doc_field_type_map = self._process_type_map(self.doc_field_types) - def get_field_type_map(self): # type: () -> Dict[str, Tuple[Field, bool]] + if self._doc_field_type_map == {}: + self._doc_field_type_map = {} + for field in self.doc_field_types: + for name in field.names: + self._doc_field_type_map[name] = (field, False) + if field.is_typed: + typed_field = cast(TypedField, field) + for name in typed_field.typenames: + self._doc_field_type_map[name] = (field, True) return self._doc_field_type_map def get_signatures(self): From e9ee39d655be2c1aff29aa3716f9f3dfbcd82006 Mon Sep 17 00:00:00 2001 From: Viktor Haag Date: Tue, 18 Jun 2019 09:37:52 -0400 Subject: [PATCH 5/5] repair white space --- sphinx/directives/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sphinx/directives/__init__.py b/sphinx/directives/__init__.py index 8a17fc0a520..f7fd6427090 100644 --- a/sphinx/directives/__init__.py +++ b/sphinx/directives/__init__.py @@ -77,10 +77,12 @@ def get_field_type_map(self): for field in self.doc_field_types: for name in field.names: self._doc_field_type_map[name] = (field, False) + if field.is_typed: typed_field = cast(TypedField, field) for name in typed_field.typenames: self._doc_field_type_map[name] = (field, True) + return self._doc_field_type_map def get_signatures(self):