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

doc index: fix attribute name in example #93

Closed
wants to merge 1 commit into from

Conversation

merwok
Copy link

@merwok merwok commented Dec 1, 2024

No description provided.

@@ -140,7 +140,7 @@ chance to grab a reference to his original function:
:linenos:

from myapp import logged_in
result = logged_in.original_func(None)
result = logged_in.original_function(None)
self.assertEqual(result['result'], 'Logged in')

That works. But it's just a little weird. Since the ``jsonify``
Copy link
Author

Choose a reason for hiding this comment

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

I also want to add a note about functools.wraps that adds func.__wrapped__, which is better than original_func as an official convention, but does not remove the point and usefulness of venusian

Copy link
Member

Choose a reason for hiding this comment

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

I have some dim memory of intentionally not using functools.wrap because it lost information of some kind, but I can't remember what it was. It may not be applicable these days, but I doubt we want to change things around to be "more correct" at the expense of possibly breaking things.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think in this particular case, the use of manually attaching original_function instead of functools.wraps was only so all the code was in the same place for didactic purposes so when someone reads the docs they don't need to know how functools.wraps works under the hood. Venusian apparently still doesn't use functools.wraps, and I still can't remember why we don't, but we shouldn't bother in any case.

Copy link
Author

Choose a reason for hiding this comment

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

I only wanted to mention it, not change the code

@merwok merwok marked this pull request as draft December 1, 2024 23:42
@@ -140,7 +140,7 @@ chance to grab a reference to his original function:
:linenos:

from myapp import logged_in
result = logged_in.original_func(None)
result = logged_in.original_function(None)
self.assertEqual(result['result'], 'Logged in')

That works. But it's just a little weird. Since the ``jsonify``
Copy link
Member

Choose a reason for hiding this comment

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

I have some dim memory of intentionally not using functools.wrap because it lost information of some kind, but I can't remember what it was. It may not be applicable these days, but I doubt we want to change things around to be "more correct" at the expense of possibly breaking things.

@mcdonc
Copy link
Member

mcdonc commented Dec 2, 2024

This was handled in #94.

@mcdonc mcdonc closed this Dec 2, 2024
@merwok merwok deleted the patch-1 branch December 2, 2024 16:56
@merwok merwok restored the patch-1 branch December 2, 2024 16:58
@merwok
Copy link
Author

merwok commented Dec 2, 2024

This was not handled in that CI PR

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