Skip to content

Conversation

@draganaurosgrbic
Copy link
Contributor

No description provided.

@draganaurosgrbic draganaurosgrbic requested a review from LalehB August 8, 2025 02:02
Copy link
Collaborator

@LalehB LalehB left a comment

Choose a reason for hiding this comment

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

Thank you @draganaurosgrbic !
I have a few suggestions:

  • The bullet list of methods for tesseract in the “Explanation of each method” section still lists the old decode(detections: list[int]) API; consider updating it to include decode_from_detection_events, decode, and decode_batch for consistency

  • Align parameter names in the decode_to_errors overload with the implementation (det_order/det_beam rather than detector_order/detector_beam) to avoid confusion when referencing the code

  • The example code that follows the new section still demonstrates decoding via decode(detections); consider showing an example using the new decode_from_detection_events or decode (syndrome array) APIs for completeness

@draganaurosgrbic
Copy link
Contributor Author

Thank you @draganaurosgrbic ! I have a few suggestions:

  • The bullet list of methods for tesseract in the “Explanation of each method” section still lists the old decode(detections: list[int]) API; consider updating it to include decode_from_detection_events, decode, and decode_batch for consistency
  • Align parameter names in the decode_to_errors overload with the implementation (det_order/det_beam rather than detector_order/detector_beam) to avoid confusion when referencing the code
  • The example code that follows the new section still demonstrates decoding via decode(detections); consider showing an example using the new decode_from_detection_events or decode (syndrome array) APIs for completeness

@LalehB Thank you. I addressed the second and third suggestions. I am not sure I understand your first suggestion. My bullet list already includes new methods (decode with new signature, and decode_batch) and the old methods (decode_from_detection_events which used to be decode but now has a different name, decode_to_errors, etc.).

@LalehB
Copy link
Collaborator

LalehB commented Aug 8, 2025

Thank you @draganaurosgrbic ! I have a few suggestions:

  • The bullet list of methods for tesseract in the “Explanation of each method” section still lists the old decode(detections: list[int]) API; consider updating it to include decode_from_detection_events, decode, and decode_batch for consistency
  • Align parameter names in the decode_to_errors overload with the implementation (det_order/det_beam rather than detector_order/detector_beam) to avoid confusion when referencing the code
  • The example code that follows the new section still demonstrates decoding via decode(detections); consider showing an example using the new decode_from_detection_events or decode (syndrome array) APIs for completeness

@LalehB Thank you. I addressed the second and third suggestions. I am not sure I understand your first suggestion. My bullet list already includes new methods (decode with new signature, and decode_batch) and the old methods (decode_from_detection_events which used to be decode but now has a different name, decode_to_errors, etc.).

you are right! Thank you.

Copy link
Collaborator

@LalehB LalehB left a comment

Choose a reason for hiding this comment

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

LGTM!
thank you Dragana!

@draganaurosgrbic draganaurosgrbic merged commit 0ebbcf8 into main Aug 8, 2025
4 checks passed
@draganaurosgrbic draganaurosgrbic deleted the update-python-readme branch August 8, 2025 03:59
@NoureldinYosri NoureldinYosri mentioned this pull request Sep 5, 2025
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