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

Dynamically assign processors and etc. #65

Closed
nishimu opened this issue Nov 14, 2011 · 3 comments
Closed

Dynamically assign processors and etc. #65

nishimu opened this issue Nov 14, 2011 · 3 comments

Comments

@nishimu
Copy link

nishimu commented Nov 14, 2011

I want to assign the processors parameter for each instance.
For example, a ImageSpec is following;

display  = ImageSpec([Adjust(contrast=1.2, sharpness=1.1),Transpose(Transpose.ROTATE_90),
        resize.Fit(300, None)], image_field='original_image',
        format='JPEG', quality=90,cache_to=my_cache_to)

and I want to change contrast, sharpness, and the Transpose method at runtime for each instance.

I found the similar request at #11 issue.
#11 expects more general way, but I don't have a good idea for #11

My idea is;

  1. change "processors" of ImageSpec() to check callable or not, like "cache_to".
  2. the callable "processors" gets parameters from instance of Model and generates processors array

Is it an effective change or not?

@matthewwithanm
Copy link
Owner

@lettertwo just asked me about this so I thought I'd let everybody know my thoughts—and let you (@nishimu) know that I'm not ignoring you!

There is definitely a use-case for this; #11 is an example. Having said that, I'm not sure I'm a fan of allowing a processors callable. The reason is that I can see a new issue being created in two months in which somebody says, "we should make the format argument a callable." And quality. Basically, the rationale for allowing processors to be a callable would be the same for every other field—including ones that we may add in the future. Furthermore, in practice, the functions used for these values would have to repeat much of the same work—in other words, what determines which processors are used is also likely to determine the format. To have to execute the same code twice is gross.

I think the idea of a callable "processors" argument may be too narrow of a solution. What you (and @koebbe) want, in effect, is to define different specs based on properties of the model. I'm still trying to figure out if there's a way to do that that fits with how we're used to working with Django fields. Ultimately, the answer may just be to allow callables for all the arguments, but I want to see if we can't come up with something better first.

Anybody have any good ideas for alternatives here? Or examples of Django libraries with fields that change behavior based on model properties?

@jokerejoker
Copy link

How about making it posible to define the ImageSpec inside the init of the model after the super init?

@matthewwithanm
Copy link
Owner

Sorry for dragging my feet on this one.

@jokerejoker Unfortunately, model fields don't really work like that. In the case of ImageSpec, we could probably get it to work, but I think the result would be without precedent and tie our hands in the future if we need to do more bookkeeping in the contribute_to_class method.

Ultimately, I think that the best option we have (or at least the best one we've been able to come up with!) is to allow callables to be used for the processors argument. And, lucky for us, @cyberdelia has done that work for us in #85! I've pulled his changes into develop with commit 3bac472.

@nishimu Sorry again for dragging my feet so long on this solution (only to finally settle on what you suggested initially). I still have my original objections but the perfect is the enemy of the good, and we've gone long enough without a solution.

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

No branches or pull requests

3 participants