Skip to content

Add the ability to add debugging notes to the result.safe decorator. #2110

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

madkopp
Copy link

@madkopp madkopp commented Apr 11, 2025

Add the ability to add debugging notes to the result.safe decorator.

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

-Refs #2107, but may not fully close it. The need to provide a list of Exceptions in order to use the add_note_on_failure keyword is a little clunky.
number

Would appreciate any help with the overloading of the safe decorator to alleviate the need to provide a list of Exceptions.

I wasn't exactly sure how you wanted me to add the changes to the change log, so I just made a "Features" section at the top of the list that isn't attached to any version. (I wasn't sure if you wanted me to go creating version numbers on your behalf or not).

Thanks for the great package!

🙏 Please, if you or your company finds dry-python valuable, help us sustain the project by sponsoring it transparently on https://github.com/sponsors/dry-python. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Generally looks good, but there are several things to add:

  • impure_safe and future_safe should be updated as well
  • Please, add a typesafety test for this
  • Test an empty string

Thank you!

@madkopp
Copy link
Author

madkopp commented Apr 12, 2025

Generally looks good, but there are several things to add:

  • impure_safe and future_safe should be updated as well
  • Please, add a typesafety test for this
  • Test an empty string

Thank you!

Thanks for the comments! I've updated the pull request to reflect the changes. Let me know if anything needs tweaking.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Just two minor comments :)

@madkopp
Copy link
Author

madkopp commented Apr 12, 2025

Ok pushed changes. Let me know if that addresses things.

@madkopp
Copy link
Author

madkopp commented Apr 15, 2025

FYI, I was looking at the code for the Result container and noticed that there's a trace property that it looks like you intend to implement at some point. If the plan is for that to carry the traceback of any errors, that could make this PR unnecessary, depending on how it's done.

@sobolevn
Copy link
Member

Yes, tracing is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants