[DOCS] Add some json serialization docstrings. #6880
Conversation
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
👇 Click on the image for a new way to code review
Legend |
| def to_json_dict(self) -> Optional[dict]: | ||
| """Returns RenderedAtomicValueGraph as a json dictionary.""" | ||
| @public_api | ||
| def to_json_dict(self) -> Optional[dict[str, JSONValues]]: |
There was a problem hiding this comment.
This looks like a potential bug. @NathanFarmer should we be calling some serialization function on self.graph?
There was a problem hiding this comment.
Yes, as it stands right now we just use Altair (vega-lite) for graphs, so this serializes them for rendering at the client.
There was a problem hiding this comment.
I have confirmed with @NathanFarmer that this is ok for renderers.
| @@ -456,7 +464,13 @@ def __repr__(self): | |||
| def __str__(self): | |||
| return json.dumps(self.to_json_dict(), indent=2) | |||
|
|
|||
| @public_api | |||
There was a problem hiding this comment.
This is the only function I didn't annotate the return type on. There is some more implementation/typing work that needs to be done to get this correct because of he current implementation.
| @@ -236,12 +238,12 @@ def assert_valid_keys(keys: Set[str], purpose: str) -> None: | |||
|
|
|||
|
|
|||
| class SerializableDictDot(DictDot): | |||
| def to_json_dict(self) -> dict: | |||
| """ | |||
| def to_json_dict(self) -> Dict[str, JSONValues]: | |||
There was a problem hiding this comment.
This is a method that needs to be implemented by subclasses. I didn't know if we considered that as public (it's not the spreadsheet) so I didn't add the decorator but I still wanted to add the docstring.
billdirks
force-pushed
the
d/dx-237/add-some-json-serialization-docstrings
branch
from
190122a
to
259d477
Compare
anthonyburdi
deleted the
d/dx-237/add-some-json-serialization-docstrings
branch
This reverts commit 6ab3bb8.
Add 36 public api docstrings. These are mostly for implementations of
to_json_dict. Docstrings were also added toconvert_to_json_serializable. I've also updated the return type of these methods except for 1 instance (see comment inline) where some implementation/typing work needs to be done that is distinct from docstring work.Definition of Done
Please delete options that are not relevant.
Thank you for submitting!