- 
                Notifications
    
You must be signed in to change notification settings  - Fork 230
 
fix(JsonFormatter): type the constructor #170
Conversation
| def __init__( | ||
| self, | ||
| *args: Any, | ||
| json_default: OptionalCallableOrStr = None, | ||
| json_encoder: OptionalCallableOrStr = None, | ||
| json_serialiser: OptionalCallableOrStr = json.dumps, | ||
| json_indent: Optional[Union[int, str]] = None, | ||
| json_ensure_ascii: bool = True, | ||
| prefix: str = "", | ||
| rename_fields: Optional[dict] = None, | ||
| static_fields: Optional[dict] = None, | ||
| reserved_attrs: Tuple[str, ...] = RESERVED_ATTRS, | ||
| timestamp: Union[bool, str] = False, | ||
| **kwargs: Any | ||
| ): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The important part is here.
| self.json_default = self._str_to_fn(json_default) | ||
| self.json_encoder = self._str_to_fn(json_encoder) | ||
| self.json_serializer = self._str_to_fn(json_serialiser) | ||
| self.json_indent = json_indent | ||
| self.json_ensure_ascii = json_ensure_ascii | ||
| self.prefix = prefix | ||
| self.rename_fields = rename_fields or {} | ||
| self.static_fields = static_fields or {} | ||
| self.reserved_attrs = dict(zip(reserved_attrs, reserved_attrs)) | ||
| self.timestamp = kwargs.pop("timestamp", False) | ||
| self.timestamp = timestamp | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| :param json_indent: indent parameter for json.dumps | ||
| :param json_ensure_ascii: ensure_ascii parameter for json.dumps | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the duplicated doc for json_indent and I moved json_ensure_ascii next to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside, typing really makes this code look ugly as hell :(
          
 Heheh, but it’s worthwhile to catch bugs. But anyway, do you know when this will roll out into a new release?  | 
    
| 
           Hi, could you consider making a new release? Currently all our apps have a version constraint   | 
    
| 
           Hi @madzak could you publish a release with this change included?  | 
    
madzak/python-json-logger#170 was included on the main branch of the upstream but never released, thus it was inadvertently released in `3.0.0`. This included the accidental renaming from US spelling to UK spelling of "serializer". This PR reverts that change. Co-authored-by: Julian Gilbey <jdg@debian.org>
Now that the package include
py.typed, mypy actually checks the type. I am usingdisallow-untyped-calls, so it fails because the constructor ofJsonFormatteris untyped. This PR fixes it.I run
toxwhich runsblack, which fixed some formatting issues.