Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Docstrings in Task/braket_simulator #976

Closed
wants to merge 2 commits into from

Conversation

shubhusion
Copy link
Contributor

Fixes #957

Returns:
BraketEmulatorTask: The current instance with the task results updated.

Example:
Copy link
Member

Choose a reason for hiding this comment

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

same as my previous comment, this needs to use a markdown code block

Copy link
Member

Choose a reason for hiding this comment

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

also can you actually run these code and copy the important printings back, e.g the printing of task and result

Copy link
Member

@weinbe58 weinbe58 Jun 10, 2024

Choose a reason for hiding this comment

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

@shubhusion Just for clarity you all you need to do:

"""

Example:
    ```python
    >>> <example code>
    >>> ...
    ```

"""

ValueError: If the task has not been submitted yet.

Example:
>>> task_spec = BraketTaskSpecification(...) # Task specification setup
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be more specific, e.g what does the specification take? and I don't think this is what user will be using. The result is called in the builder call chain.

int: The number of shots.

Example:
>>> task_spec = BraketTaskSpecification(...) # Task specification setup
Copy link
Member

Choose a reason for hiding this comment

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

same, you can't ask a user to construct task specification directly

Copy link

codecov bot commented May 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.90%. Comparing base (e0779e8) to head (51304e2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #976   +/-   ##
=======================================
  Coverage   89.90%   89.90%           
=======================================
  Files         110      110           
  Lines        8131     8131           
=======================================
  Hits         7310     7310           
  Misses        821      821           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Roger-luo Roger-luo left a comment

Choose a reason for hiding this comment

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

I think in general I think you need to run the emulator to come up with better examples, e.g demonstrate what intermediate result people will get by calling run? what next steps they can do after calling run? I also leave a few more specific comment above. I don't think you should have things like metadata show up in an example that is not very idiomatic.

task_ir: BraketTaskSpecification
metadata: Dict[str, ParamType]
task_result_ir: Optional[QuEraTaskResults] = None

def _geometry(self) -> Geometry:
"""
Copy link
Member

Choose a reason for hiding this comment

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

This is a private function, and I'm doing duplicated reviews on the same issue. Please work on one of your PRs first before requesting multiple reviews thank you.

@weinbe58 weinbe58 closed this Aug 2, 2024
@shubhusion shubhusion deleted the patch-8 branch August 5, 2024 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve docstrings
3 participants