Skip to content

Conversation

@Aniketsy
Copy link
Contributor

@Aniketsy Aniketsy commented Sep 4, 2025

This PR enhances the docstrings for the PQC and Expectation layers to comply with TensorFlow documentation standards.

Updates:
Added structured Args, Input shape, and Output shape sections to the class level docstrings.
Updated the call method docstrings with explicit Args and Returns sections, including types and tensor shapes.

Please let me know if this fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thankyou!

@Aniketsy Aniketsy changed the title Improve Documentation for PQC and Expectation Layers (TensorFlow Quantum)(GH#238) Improve Documentation for PQC and Expectation Layers (GH#238) Sep 4, 2025
@Aniketsy
Copy link
Contributor Author

Aniketsy commented Sep 4, 2025

Hi @mhucka
I’ve pushed the changes for this issue and would like to request your review. I wasn’t able to run pre-commit run locally, so there might be some formatting errors. Please let me know if anything needs adjustment, and I’ll be happy to fix it.

@mhucka
Copy link
Member

mhucka commented Sep 12, 2025

@Aniketsy Thank you for your work. There's one minor format issue I see right off: we're trying to keep to 80 character line lengths (see the .editorconfig configuration file). This is the reason why the CI check for formatting is failing (https://github.com/tensorflow/quantum/actions/runs/17688294620/job/50277418161?pr=895). Hopefully this won't be hard to fix up.

@Aniketsy
Copy link
Contributor Author

@mhucka Thanks! I’ve fixed the format error and updated the PR. I’d also love to contribute to other issues, but I understand they are already assigned. Please feel free to point out any issues I could work on I’d be happy to take them on.

@mhucka
Copy link
Member

mhucka commented Sep 13, 2025

@Aniketsy Thank you for your interest. I did a round of updates on all the issues and unassigned a couple. You don't necessarily have to take these on, but it would be hard to find which ones were unassigned, so I'll just list them here:

Copy link
Member

@mhucka mhucka left a comment

Choose a reason for hiding this comment

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

This looks good. A couple of trivial requests. These are not about your changes but about the rest of the file, and I thought since you're fixing up the docstrings in this file, these could be done along the way:

  • Super trivial: the docstring for the class Expectation has an extra empty line on line 229, which by docstring conventions shouldn't be there. Could you remove it?
  • The docstring for __init__ lacks documentation for the kwargs argument. Could you add a description as part of this PR?

Copy link
Member

@mhucka mhucka left a comment

Choose a reason for hiding this comment

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

Also, make sure to update the branch using the button in the GitHub interface. (If I do it, it may add my name to the PR, which I feel would be unfair.)

@Aniketsy
Copy link
Contributor Author

Thanks! I’ve applied the changes based on your suggestions.

@Aniketsy
Copy link
Contributor Author

@mhucka Please review these changes when you have a chance.

@mhucka mhucka self-assigned this Dec 8, 2025
@mhucka
Copy link
Member

mhucka commented Dec 8, 2025

@Aniketsy Thank you for your work on this. This looks good. I made a small change to one of the docstrings (it might even have been in the original and not your additions); for the sake of expediency, I pushed it to your branch and will approve the PR so that it can be merged.

Copy link
Member

@mhucka mhucka left a comment

Choose a reason for hiding this comment

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

LGTM

@mhucka mhucka enabled auto-merge (squash) December 8, 2025 22:40
@mhucka mhucka merged commit 65ec707 into tensorflow:master Dec 8, 2025
10 checks passed
@Aniketsy
Copy link
Contributor Author

Aniketsy commented Dec 9, 2025

Thanks for updating the changes and merging.

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.

2 participants