Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ posthog-analytics
pyrightconfig.json
.env
.DS_Store
posthog-python-references.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add newline at end of file to follow Git best practices

80 changes: 80 additions & 0 deletions docs/doc_constant.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an RFC that states we should use the bin folder for scripts. Can you add that there instead? I'd be happy with a bin/docs executable, and then maybe a bin/docs folder with your helper scripts. How does that sound?

I'm looking for the RFC and can link it to you if I find it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes ofc, let me go through suggestions. thanks as always for sitting through reviews of code like this, my eyes started watering near the end of this.

Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
#!/usr/bin/env python3
"""
Constants for PostHog Python SDK documentation generation.
"""

# Types that are built-in to Python and don't need to be documented
NO_DOCS_TYPES = [
"Client",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: 'Client' probably shouldn't be in NO_DOCS_TYPES since it's a core SDK class that needs documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant this is only for object types. We don't need to self-reference.

"any",
"int",
"float",
"bool",
"dict",
"list",
"str",
"tuple",
"set",
"frozenset",
"bytes",
"bytearray",
"memoryview",
"range",
"slice",
"complex",
"Union",
"Optional",
"Any",
"Callable",
"Type",
"TypeVar",
"Generic",
"Literal",
"ClassVar",
"Final",
"Annotated",
"NotRequired",
"Required",
"void",
"None",
"NoneType",
"object",
"Unpack",
"BaseException",
"Exception",
]

# Documentation generation metadata
DOCUMENTATION_METADATA = {
"hogRef": "0.1",
"slugPrefix": "posthog-python",
"specUrl": "https://github.com/PostHog/posthog-python"
}

# Docstring parsing patterns for new format
DOCSTRING_PATTERNS = {
"examples_section": r'Examples:\s*\n(.*?)(?=\n\s*\n\s*Category:|\Z)',
"args_section": r'Args:\s*\n(.*?)(?=\n\s*\n\s*Examples:|\n\s*\n\s*Details:|\n\s*\n\s*Category:|\Z)',
"details_section": r'Details:\s*\n(.*?)(?=\n\s*\n\s*Examples:|\n\s*\n\s*Category:|\Z)',
"category_section": r'Category:\s*\n\s*(.+?)\s*(?:\n|$)',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: These regex patterns don't handle nested sections properly. Consider using a more robust parsing approach or adding negative lookaheads to prevent incorrect matches

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, but we don't expect weird nesting, we'd rather fix the doc string

"code_block": r'```(?:python)?\n(.*?)```',
"param_description": r'^\s*{param_name}:\s*(.+?)(?=\n\s*\w+:|\Z)',
"args_marker": r'\n\s*Args:\s*\n',
"examples_marker": r'\n\s*Examples:\s*\n',
"details_marker": r'\n\s*Details:\s*\n',
"category_marker": r'\n\s*Category:\s*\n'
}

# Output file configuration
OUTPUT_CONFIG = {
"filename": "posthog-python-references.json",
"indent": 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add a path variable for output directory instead of just filename to make the output location configurable

}

# Documentation structure defaults
DOC_DEFAULTS = {
"showDocs": True,
"releaseTag": "public",
"return_type_void": "void",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using 'None' instead of 'void' for return_type_void to align with Python conventions

Suggested change
"return_type_void": "void",
"return_type_void": "None",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be None like the bot is suggesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah probably, this is a left over from the TS specs. void isn't a thing here

"max_optional_params": 3
}
Loading
Loading