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

[Unitary hack] Improve Docstrings in "task/batch" #972

Closed
wants to merge 7 commits into from

Conversation

shubhusion
Copy link
Contributor

Fixes #957

Improve function docstrings and Remove markdown errors

@shubhusion shubhusion changed the title Improve Docstrings in "task/batch" [Unitary hack] Improve Docstrings in "task/batch" May 31, 2024
Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

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

I appreciate the contribution to the code base but I think it would be a bit beyond the scope of the unitary hack issue. If you want to contribute a change to something other than the docs we should have this be a separate PR.

Comment on lines -303 to -327
def __str__(self) -> str:
output = ""
for task_index, task_error in self.task_errors.items():
output += (
f"Task {task_index} failed to submit with error: "
f"{task_error.exception_type}\n"
f"{task_error.stack_trace}"
)

return output


@BatchErrors.set_serializer
def _serialize(self: BatchErrors) -> Dict[str, List]:
return {
"task_errors": [
(task_number, task_error)
for task_number, task_error in self.task_errors.items()
]
}


@BatchErrors.set_deserializer
def _deserialize(obj: dict) -> BatchErrors:
return BatchErrors(task_errors=OrderedDict(obj["task_errors"]))
Copy link
Member

Choose a reason for hiding this comment

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

why did you delete this code? this is making the CI fail. Please add this code back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize I made a mistake while merging conflicts. I was updating the docstrings as part of the unitary hack issue. I will resolve it. Thanks for correcting me.

Copy link
Member

Choose a reason for hiding this comment

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

No problem, if you need help setting up your development environment take a look at our docs page on that: https://bloqade.quera.com/latest/contributing/developing-bloqade/

Copy link
Member

Choose a reason for hiding this comment

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

@shubhusion can I close this PR? I haven't seen any updates on it

@weinbe58 weinbe58 closed this Aug 2, 2024
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