unrecognised_fields default value vs empty container

[jku]

I believe this code currently does the unexpected thing:

custom = {}
role = Role([], 1, custom)
custom["customfield"] = "value"
assert "customfield" in role.unrecognized_fields

As in the roles unrecognised fields now does not include "customfield". This is because the initialization self.unrecognized_fields = unrecognized_fields or {} acts surprisingly when the input container is empty. We should check for None instead of falsyness.

Same thing applies throughout the API where unrecognized fields are used.

[abs007]

Hi, I was going through metadata.py and I saw that a few initializations were type annotated like in: self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {}

but some are without the annotation like in, self.unrecognized_fields = unrecognized_fields or {}. Could you give some context into this?

[jku]

That's a good question: The context is that mypy (the static type checker we run as part of the linting process) can usually figure out the types of variables from context but sometimes it needs help: To use {} as a example, it is an empty container so mypy can't figure out what the key type is, but we might know it.

Now that you brought it up: the explicit annotations are possibly not required now that Mapping is not used (thanks for the patch btw!)... If mypy does not complain when you run tox, removing the annotation completely in these cases self.unrecognized_fields: Dict[str, Any] = unrecognized_fields or {} is fine by me: then the actual line would be something like

self.unrecognized_fields = unrecognized_fields if unrecognized_fields is not None else {}

---

If you are wondering how static typing sees a specific variable, you have a couple of options:

• use VS Code or other IDE that can show the types on mouse over: this is a fantastic helper
• let mypy tell you: if it can't figure out a variable it will complain when you run tox ... additionally if you are really interested, there is a reveal_type() test you can do to ask mypy what it thinks a type is. If you have a file like this

from typing import Dict

d1:Dict[str, int] = {}
d2 = {"a": 1}
d3 = {}

reveal_type(d1)
reveal_type(d2)
reveal_type(d3)

now running mypy file.py will A) give you an error because it can't infer type of d3 and B) will print out the types it knows or has inferred.

[jku]

self.unrecognized_fields = unrecognized_fields if unrecognized_fields is not None else {}

that line gets really long so maybe a plain old if-clause works better: I'll leave it to you to to find something that passes the linter :)

[abs007]

mypy isn't able to infer the type for d3 so similarly won't mypy complain if we remove the annotation in self.unrecognized_fields: Dict[str, Any] = unrecognized_fields or {}

[abs007]

self.unrecognized_fields = unrecognized_fields if unrecognized_fields is not None else {}

that line gets really long so maybe a plain old if-clause works better: I'll leave it to you to to find something that passes the linter :)

Do you mean with an if-else over multiple lines?