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

Added some methods and functions to option and result #172

Merged

Conversation

brendanmaguire
Copy link
Contributor

  • Convert Options to Results and vice versa
  • Added Option#to_optional for easy convertion to Python's Optional[T]
  • Added a Result#swap method
  • Added tests for all of the above

Also:

  • Fixed some documentation for Result#default_values and Result#default_with

@brendanmaguire
Copy link
Contributor Author

Hey @dbrattli . Are you accepting PRs to this project? If so and you would be interested in merging this functionality, I can update my branch. If not then I can close the PR.

@@ -28,6 +28,7 @@

from typing_extensions import TypeAlias, TypeGuard

from . import option
Copy link
Owner

@dbrattli dbrattli Oct 26, 2023

Choose a reason for hiding this comment

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

Not happy with option import result and result importing option. Can we use if TYPE_CHECKING: here to avoid getting circular dependencies in the running code?

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'll take a look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I've updated the PR with your suggestion.

* Convert `Option`s to `Result`s and vice versa
* Added `Option#to_optional` for easy convertion to Python's `Optional[T]`
* Added a `Result#swap` method
* Added tests for all of the above

Also:
* Fixed some documentation for `Result#default_values` and `Result#default_with`
@brendanmaguire brendanmaguire force-pushed the new-option-and-result-methods branch from fd39da5 to b6f2fea Compare October 29, 2023 16:44
Copy link
Owner

@dbrattli dbrattli left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks for contributing!

@dbrattli dbrattli merged commit 8b2b5a0 into dbrattli:main Nov 2, 2023
@brendanmaguire brendanmaguire deleted the new-option-and-result-methods branch November 3, 2023 12:48
@brendanmaguire
Copy link
Contributor Author

Looks great. Thanks for contributing!

Thank you for the great library 🙂

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