feat: add summary page for Classes and Modules by dandhlee · Pull Request #361 · googleapis/sphinx-docfx-yaml

dandhlee · 2024-03-27T08:34:06Z

Curate summary pages given the entirety of the library content. Starting with Classes / Modules which are similar, then will expand to other types (Methods & Functions, and Properties & Attributes) in a followup.

Goes through all of the content, then extracts the necessary information, creating a separate summary_class.yml file. Adding as a separate top level entry. See live example: https://cloud.google.com/python/docs/reference/bigframes/latest/summary_class

Confirmed locally that the same page still gets produced, which was based off of #298, this PR just makes the code a bit more production quality.

summary_overview page is referenced but will have templates that live in the client libraries. See https://docs.google.com/document/d/1FmAOv9ald2W2Set8jPzN-gwQoE992LCWSu40KBVON28/edit?resourcekey=0-JLhux0549oJustMl46l3zA&tab=t.0 for design.

Test cases would be added for goldens, however it is currently disabled.

Towards b/263399076

Tests pass
Appropriate changes to README are included in PR

dansaadati · 2024-03-28T04:14:04Z

+
+        file_path_to_use = os.path.join(normalized_outdir, file_name)
+        with open(file_path_to_use, "w") as summary_file_obj:
+            summary_file_obj.write("### YamlMime:UniversalReference\n")


Nit: set a constant for the docfx YAML type string

dansaadati · 2024-03-28T05:02:22Z

+    CLASS: CLASS,
+}
+# Construct a mapping of name and content for each unique summary type entry.
+_ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE = {


Can this be converted to a dataclass? Something like:

class SummaryEntry: name: str summary: str

For now it seems like there's just one summary type CLASS, so I think this information can be captured in just one data class. This would allow us to not use i-indexing below, which I think makes this more readable.

It's a bit tricky, I need to keep track of all the entries that's been submitted already. Either we need to keep an extra list, otherwise converting this to a dataclass we'd lose being able to have a sequence of information, instead we'll have Sequence[SummaryEntry] and not be able to easily go through the info :/

dansaadati · 2024-03-28T05:08:53Z

    DEPRECATED: 'deprecated',
 }

+_SUMMARY_TYPE_BY_ITEM_TYPE = {


Nit: Maybe use _SUMMARY_GROUP instead? The phrasing can get a little bit confusing because you're working with Python types as well.

This was a preferred style for mappings (summary_by_item[item] gives summary) - I could also just omit the types if that makes it better

dansaadati · 2024-03-28T05:14:16Z

+    uid = yaml_data.get("uid", "")
+    item_to_add = uid if summary_type == CLASS else f"{uid}-summary"
+
+    if item_to_add not in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0]:


I think you can avoid this check if your use a Set for your uids.

I think the problem was that I ran into the same items twice, not that the UIDs are not unique :/ I was seeing duplicate entries for each entry, but the UIDs are definitely unique (google.cloud.package.module.item)

dansaadati · 2024-03-28T05:19:45Z

+              continue
+
+            _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][1].append(
+                _find_summary_details(entry, summary_type, cgc_url)


Nit: I think because the return value of _find_summary_details is only used to update the global mapping, I would just append the summary detail dict within the function.

Done! Good point.

dansaadati · 2024-03-28T05:25:07Z

+            ],
+        }
+    )
+    cgc_url = (


Nit: Move this below writing the toc file.

dansaadati · 2024-03-28T05:34:31Z

+                'langs': ['python'],
+                'type': 'package',
+                'summary': f'Summary of entries of {entry_name} for {library_name}.',
+                'children': children_name_and_summary_content[0],


Nit: consider sorting by class/module name here.

It's already sorted by UID!

dansaadati · 2024-03-28T05:42:28Z

+
+
+def _render_summary_content(
+    children_name_and_summary_content: Sequence[Sequence[str]],


Could be mistaken here, but isn't this type Sequence[Sequence[ str | dict ] or Sequence[Sequence[ str | _yaml_type_alias ] ? The first sequence contains the list of uids, the second sequence contains the dict of YAML fields for references.

You're right. Done.

dansaadati · 2024-03-28T05:53:48Z

+                'langs': ['python'],
+                'type': 'package',
+                'summary': f'Summary of entries of {entry_name} for {library_name}.',
+                'children': children_name_and_summary_content[0],


Doesn't this need to be converted into a YAML list?
Something like:

reduce(lambda x, y: y + '\n - ' + x , children_name_and_summary_content[0])

Nope, that gets handled already!

dandhlee

Verified that the changes did not affect produced result. Please take a look again!

dandhlee · 2024-03-29T01:39:26Z

+
+        file_path_to_use = os.path.join(normalized_outdir, file_name)
+        with open(file_path_to_use, "w") as summary_file_obj:
+            summary_file_obj.write("### YamlMime:UniversalReference\n")


dandhlee · 2024-03-29T02:31:10Z

+                'langs': ['python'],
+                'type': 'package',
+                'summary': f'Summary of entries of {entry_name} for {library_name}.',
+                'children': children_name_and_summary_content[0],


It's already sorted by UID!

dandhlee · 2024-03-29T02:32:54Z

+
+
+def _render_summary_content(
+    children_name_and_summary_content: Sequence[Sequence[str]],


You're right. Done.

dandhlee · 2024-03-29T19:03:06Z

+    uid = yaml_data.get("uid", "")
+    item_to_add = uid if summary_type == CLASS else f"{uid}-summary"
+
+    if item_to_add not in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0]:


I think the problem was that I ran into the same items twice, not that the UIDs are not unique :/ I was seeing duplicate entries for each entry, but the UIDs are definitely unique (google.cloud.package.module.item)

dandhlee · 2024-03-29T19:07:38Z

+    CLASS: CLASS,
+}
+# Construct a mapping of name and content for each unique summary type entry.
+_ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE = {


It's a bit tricky, I need to keep track of all the entries that's been submitted already. Either we need to keep an extra list, otherwise converting this to a dataclass we'd lose being able to have a sequence of information, instead we'll have Sequence[SummaryEntry] and not be able to easily go through the info :/

dandhlee · 2024-03-29T19:11:39Z

    DEPRECATED: 'deprecated',
 }

+_SUMMARY_TYPE_BY_ITEM_TYPE = {


This was a preferred style for mappings (summary_by_item[item] gives summary) - I could also just omit the types if that makes it better

dandhlee · 2024-03-29T19:12:11Z

+                'langs': ['python'],
+                'type': 'package',
+                'summary': f'Summary of entries of {entry_name} for {library_name}.',
+                'children': children_name_and_summary_content[0],


Nope, that gets handled already!

dandhlee · 2024-03-29T19:12:52Z

+            ],
+        }
+    )
+    cgc_url = (


dandhlee · 2024-03-29T19:14:30Z

+              continue
+
+            _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][1].append(
+                _find_summary_details(entry, summary_type, cgc_url)


Done! Good point.

feat: add summary page for Classes and Modules

6729683

product-auto-label Bot added the size: m Pull request size is medium. label Mar 27, 2024

dandhlee and others added 2 commits March 27, 2024 04:35

Merge branch 'main' into add_summary_page

a9a1e26

fix: update toc yaml alias to just be yaml alias

f971a38

dandhlee marked this pull request as ready for review March 27, 2024 08:47

dandhlee requested review from a team and dansaadati March 27, 2024 08:47

dansaadati suggested changes Mar 28, 2024

View reviewed changes

feat: address review comments

93980c0

dandhlee commented Mar 29, 2024

View reviewed changes

dandhlee requested a review from dansaadati March 29, 2024 19:18

dansaadati approved these changes Mar 29, 2024

View reviewed changes

dandhlee merged commit e56c3a7 into main Mar 29, 2024

dandhlee deleted the add_summary_page branch March 29, 2024 20:05

release-please Bot mentioned this pull request Mar 29, 2024

chore(main): release 3.1.0 #362

Merged

dandhlee mentioned this pull request Mar 29, 2024

feat: add summary page support for methods and properties #363

Merged

2 tasks



		def _render_summary_content(
		children_name_and_summary_content: Sequence[Sequence[str]],

Conversation

dandhlee commented Mar 27, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dandhlee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants