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

Allow to use __caching_id__ rather than __repr__ as an object caching key #123

Merged
merged 1 commit into from Oct 25, 2019

Conversation

jd
Copy link
Contributor

@jd jd commented Jun 24, 2019

This allows users to implement caching_id as a class method to override the
default repr usage that does not suit all uses cases.

… key

This allows users to implement __caching_id__ as a class method to override the
default `repr` usage that does not suit all uses cases.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 79.224% when pulling adfde26 on jd:alternate-repr into 3decee7 on sh4nks:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 79.224% when pulling adfde26 on jd:alternate-repr into 3decee7 on sh4nks:master.

@@ -71,6 +71,14 @@ def get_arg_default(f, position):
return arg_def if arg_def != inspect.Parameter.empty else None


def get_id_func(obj):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this separate function is needed here? As i see you only use this value within get_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For future reuse and ease readability?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I simply canʼt think of a use case when you need the id generating function but not the ID. And this way you add an unnecessary function call to a oneliner with no significant improvement on readability. How about something like this?

def get_id(obj):
    id_func = getattr(obj, '__caching_id__', repr)

    return id_func(obj)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂

@sh4nks sh4nks merged commit 49ff08b into pallets-eco:master Oct 25, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants