Skip to content

Add type stub file for _ds C++ extension module - #137

Merged
hzhangxyz merged 2 commits into
mainfrom
copilot/write-ds-pyi-in-apyds
Dec 20, 2025
Merged

Add type stub file for _ds C++ extension module#137
hzhangxyz merged 2 commits into
mainfrom
copilot/write-ds-pyi-in-apyds

Conversation

Copilot AI commented Dec 20, 2025

Copy link
Copy Markdown
Contributor

Add _ds.pyi stub file to provide type hints for the pybind11 C++ extension module.

Changes

  • Type stubs for all extension classes: String, Variable, Item, List, Term, Rule, Search
  • Static method signatures: ground(), match(), rename() for Term and Rule classes
  • Enum definitions: Term.Type with Variable, Item, List, Null values
  • Special methods: len, getitem, init with proper type annotations

Benefits

  • IDE autocomplete and type checking support for the C++ extension
  • Static analysis with mypy and other type checkers
  • Improved developer experience with inline documentation

Example

from apyds import _ds

# Type checker now understands the extension module interface
term: _ds.Term = _ds.Term.from_string("(f `x a)", 1000)
result: _ds.Term | None = _ds.Term.ground(term, dictionary, "scope", 1000)

The stub file follows PEP 561 conventions and complements the existing py.typed marker in the package.

Original prompt

This section details on the original issue you should resolve

<issue_title>Write a _ds.pyi in /apyds</issue_title>
<issue_description></issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Add _ds.pyi file in /apyds directory Add type stub file for _ds C++ extension module Dec 20, 2025
Copilot AI requested a review from hzhangxyz December 20, 2025 06:27
@hzhangxyz
hzhangxyz marked this pull request as ready for review December 20, 2025 06:27
Copilot AI review requested due to automatic review settings December 20, 2025 06:27
@hzhangxyz
hzhangxyz merged commit 52fb2a5 into main Dec 20, 2025
57 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a comprehensive type stub file (_ds.pyi) for the C++ extension module built with pybind11, enabling IDE autocomplete, static type checking with mypy, and improved developer experience.

Key Changes

  • Complete type annotations for all 7 C++ extension classes (String, Variable, Item, List, Term, Rule, Search)
  • Static method signatures for serialization, deserialization, and operations like ground(), match(), and rename()
  • Nested Term.Type enum definition with proper values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apyds/_ds.pyi

Args:
rule: The rule to rename.
prefix_and_suffix: A rule with two inner lists for prefix and suffix.

Copilot AI Dec 20, 2025

Copy link

Choose a reason for hiding this comment

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

The documentation states the prefix_and_suffix parameter is "A rule with two inner lists for prefix and suffix" but the type signature shows it as Rule. Based on the usage in rule_t.py (line 120), this should be more clearly documented as "A Rule object whose conclusion is a list containing two inner lists".

Suggested change
prefix_and_suffix: A rule with two inner lists for prefix and suffix.
prefix_and_suffix: A Rule object whose conclusion is a list containing two inner lists (prefix and suffix).

Copilot uses AI. Check for mistakes.
Comment thread apyds/_ds.pyi

Args:
term: The term to rename.
prefix_and_suffix: A term with two inner lists for prefix and suffix.

Copilot AI Dec 20, 2025

Copy link

Choose a reason for hiding this comment

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

The documentation describes the prefix_and_suffix parameter as "A term with two inner lists for prefix and suffix", but based on the usage in term_t.py (line 106), this should be more precisely documented as "A Term representing a list containing two inner lists, where each inner list contains 0 or 1 item for the prefix and suffix respectively".

Suggested change
prefix_and_suffix: A term with two inner lists for prefix and suffix.
prefix_and_suffix: A Term representing a list containing two inner lists, where each inner list
contains 0 or 1 item for the prefix and suffix respectively.

Copilot uses AI. Check for mistakes.
Comment thread apyds/_ds.pyi
Comment on lines +459 to +462
term_1: The first term to match.
term_2: The second term to match.
scope_1: The scope for the first term.
scope_2: The scope for the second term.

Copilot AI Dec 20, 2025

Copy link

Choose a reason for hiding this comment

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

The scope_1 and scope_2 parameters lack clear documentation about their purpose. Based on the usage in term_t.py (line 96), these are scope identifiers used to distinguish variables from different terms during unification. Consider adding more detail about how these scopes work.

Copilot uses AI. Check for mistakes.
Comment thread apyds/_ds.pyi

Args:
term: The term to ground.
dictionary: A term representing a dictionary (list of pairs).

Copilot AI Dec 20, 2025

Copy link

Choose a reason for hiding this comment

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

The documentation describes the dictionary parameter as "A term representing a dictionary (list of pairs)" but doesn't clarify the structure. Based on the usage in term_t.py (lines 57-58), this should be more clearly documented as "A Term representing a list of pairs, where each pair contains a variable and its substitution value".

Suggested change
dictionary: A term representing a dictionary (list of pairs).
dictionary: A Term representing a list of pairs, where each pair
contains a variable and its substitution value.

Copilot uses AI. Check for mistakes.
Comment thread apyds/_ds.pyi

Args:
rule: The rule to ground.
dictionary: A rule representing a dictionary (list of pairs).

Copilot AI Dec 20, 2025

Copy link

Choose a reason for hiding this comment

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

The documentation states the dictionary parameter is "A rule representing a dictionary (list of pairs)" but the type signature shows it as Rule. However, examining the usage examples in rule_t.py (lines 65 and 73), dictionaries are actually Rules with only conclusions. The documentation should clarify that this is a Rule object whose conclusion is a list of pairs, not a "rule representing a dictionary".

Suggested change
dictionary: A rule representing a dictionary (list of pairs).
dictionary: A Rule whose conclusion is a list of pairs used as a substitution dictionary.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write a _ds.pyi in /apyds

3 participants