-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add label/description decorator for "cleaner" code #115
Comments
neat. It bugs me too but I was copying how a lot Django admin stuff does. I haven't checked out those docs in awhile though. I worry about "There should be one-- and preferably only one --obvious way to do it." from PEP 20 These assignments create a lot of annoying mypy errors too. |
if the Django admin has this pattern now then ya we should definitely do it! |
With regards to the Zen of Python, I think Django has been lost from the beginning. It's filled with (meta) magic all over so there's no real clean way anymore. A cleaner way to do it would have been similar to argparse: https://docs.python.org/3/library/argparse.html#example actions = Actions()
@actions.add_action(label=label, ...)
def my_action(self, request, obj):
... That would still work regular functions too if you wanted: actions.add_action(label=label, ...)(some_function) Or perhaps better: actions.add_action(some_function, label=label, ...) Note that I used keyword arguments for the @actions.add_action
def my_action(self, request, obj):
... Just a fyi, I'm Dutch so I should be good with PEP20 ;) |
It doesn't look like Django is doing something like this yet: https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#django.contrib.admin.ModelAdmin.list_display I still see def set_attributes(**kwargs):
def _set_attributes(function):
for key, value in kwargs.items():
setattr(function, key, value)
return function
return _set_attributes |
Great thoughts @wolph. It sounds like a universal-ish decorator would be a better way to solve this than to scope it to just this package. |
That's fair I guess, I'll add it to my Django Utils library in that case: https://pypi.org/project/django-utils2/ |
Well... great minds think alike. It seems that the Django development release has this feature: https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.display |
While we're still stuck with older Django versions, the python-utils package now has this included. This is a pure-python package I maintain which is available through the repo in most BSD/Linux distributions. |
Unfortunately this decorator cannot be used with |
Interesting! I'll look into seeing what it'll take to get it to work... one day. I made this issue bigger so it stands out more |
Just wanted to add my support - now that Django has an @admin.display() decorator for "admin actions", it sure would be nice if django-object-actions offered a similar decorator for object actions. Would look better / more consistent. |
Reference: crccheck#115
New release on pypi? |
There was some sort of cleanup thing I was going to do before a release but now I can't remember. So I'll just hit the button to release! |
Well the CI job is broken, but I was able to publish it manually https://pypi.org/project/django-object-actions/4.1.0/ |
Perhaps it's something that only irks me, but I find that this syntax quickly becomes readable after the function goes beyond a few lines:
I'm currently using this little decorator to take care of the labels and description which works great:
Usage is like this:
The text was updated successfully, but these errors were encountered: