Add support for multiple path converters on urls.path()

2019-08-30 Thread Marco Silva
Hello,

I'm on the process of migrating an app to django >2 and walked into this 
potencial new feature while converting the URLs.

Some modeles use a "display" id, like TICKET-2019-002, but it's pk is 
actually a uuid, so the url would ideally support both.

I've created a custom converter to verify the correct value, therefore I 
can use:
`path('api/ticket/', TicketDetailView.as_view())`

but if i want to also expose the api with the uuid, I have to add another 
urlpattern with:
`path('api/ticket/', TicketDetailView.as_view())`

my proposal would be to add support for this:
`path('api/ticket/', TicketDetailView.as_view())`


The concrete example is a different use case, its a "common" view where you 
must specify the type first, so basically:
`path('api///assign', 
GenericAssignView.as_view())`


>From what i've seen on the API, wouldn't be much work to add regex groups 
on the djanjo.urls.resolvers._route_to_regex method, but it also returns a 
converters dictionary with a mapping of parameters(the ticket, or 
refecence_nr) with a single converter, and that could cause problems 
somewhere...

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/706ffd8d-06b9-4c21-82d9-ee1c69e7aa69%40googlegroups.com.


Re: Add support for multiple path converters on urls.path()

2019-09-02 Thread Marco Silva
Hey,

Thank you for your feedback.

Making a new URL definition is a good approach, even though the example I 
gave would require a few extra lines, that may be more readable than 
squeezing everything together.

Regarding the custom converter, I already have custom converters in place, 
and I don't think making a new one for every combination would be better, 
my initial goal was to reuse those converters as they can be properly 
tested.

Anyway, multiple url definitions it is, thank you all again.



On Saturday, August 31, 2019 at 10:32:35 AM UTC+1, Adam Johnson wrote:
>
> Hi Marco,
>
> Thanks for writing a clear, short proposal.
>
> This could be neat, but I'm against it for these reasons:
>
>- It's extra syntax - the advantage of the new URL syntax is that it's 
>very simple and easy to pick up. It's copied from Flask, which also 
> doesn't 
>support a "multiple converter" syntax  ( 
>https://flask.palletsprojects.com/en/1.1.x/quickstart/#routing ). 
>Instead, you're expected to define multiple routes if that's what you want.
>- It is only a shortcut - the behaviour you want can be achieve with 
>multiple URL definitions. This is clearer in terms of readability and 
>precedence (top to bottom). Understanding precedence could be a problem in 
>particular for a multi-converter pattern like "//" - 
>does "int a, str b" have precedence over "str a, int b", or the other way 
>round?
>- You can define a custom converter right now, as Richard suggests.
>
> Thanks for taking the time to write to the mailing list though,
>
> Adam
>
> On Fri, 30 Aug 2019 at 18:30, Richard  > wrote:
>
>> A possibility without changes to Django is to create a generic converter 
>> and apply it to each model:
>>
>> class ModelUUIDConverter:
>>regex= r'[a-fA-F0-9]{32}'
>>def __init__(self, model):
>>self.model = model
>>def to_python(self, value):
>>  return self.model.objects.filter(Q(uuid=value)|Q(display_id=value).
>> get()
>>def to_url(self,value):
>>  return str(value)
>>
>> urls.register_converter(lambda: ModelUUIDConverter(TicketModel), 
>> 'ticket_or_uuid')
>>
>> path('api/',TicketDetailView.as_view())
>>
>>
>>
>> On Friday, 30 August 2019 11:12:08 UTC-6, Marco Silva wrote:
>>>
>>> Hello,
>>>
>>> I'm on the process of migrating an app to django >2 and walked into this 
>>> potencial new feature while converting the URLs.
>>>
>>> Some modeles use a "display" id, like TICKET-2019-002, but it's pk is 
>>> actually a uuid, so the url would ideally support both.
>>>
>>> I've created a custom converter to verify the correct value, therefore I 
>>> can use:
>>> `path('api/ticket/', TicketDetailView.as_view())`
>>>
>>> but if i want to also expose the api with the uuid, I have to add 
>>> another urlpattern with:
>>> `path('api/ticket/', TicketDetailView.as_view())`
>>>
>>> my proposal would be to add support for this:
>>> `path('api/ticket/', TicketDetailView.as_view())`
>>>
>>>
>>> The concrete example is a different use case, its a "common" view where 
>>> you must specify the type first, so basically:
>>> `path('api///assign', 
>>> GenericAssignView.as_view())`
>>>
>>>
>>> From what i've seen on the API, wouldn't be much work to add regex 
>>> groups on the djanjo.urls.resolvers._route_to_regex method, but it also 
>>> returns a converters dictionary with a mapping of parameters(the ticket, or 
>>> refecence_nr) with a single converter, and that could cause problems 
>>> somewhere...
>>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-d...@googlegroups.com .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/d7cb9137-98bf-4db0-993b-538000321c65%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/django-developers/d7cb9137-98bf-4db0-993b-538000321c65%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
>
>
> -- 
> Adam
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8bb84ddf-cd1d-4461-92d7-fb7fc601a4d4%40googlegroups.com.


Re: Proposal: make Model __unicode__() default to self.name

2017-04-07 Thread Marco Silva
I noticed this on the init

def __init__(self, *args, **kwargs):
 # Alias some things as locals to avoid repeat global lookups
 cls = self.__class__

maybe you should change it to self.cls??
Try to submit a PR to the open ticket.

segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>
> So does this patch seem fine ? 
>
> diff --git a/django/db/models/base.py b/django/db/models/base.py
> index 3c1cb9a..f58e12b 100644
> --- a/django/db/models/base.py
> +++ b/django/db/models/base.py
> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>  return '<%s: %s>' % (self.__class__.__name__, u)
>  
>  def __str__(self):
> -return '%s object' % self.__class__.__name__
> +return '%s object pk=%s' % (self.__class__.__name__, 
> self._get_pk_val())
>  
>  def __eq__(self, other):
>  if not isinstance(other, Model):
>
>
>
>
> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>
>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may 
>> default to generating a pk before save, so it might just be best to show 
>> the actual current pk value
>>
>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  wrote:
>>
>>> So what should the final __str__ show: Should it be 'ClassName object 
>>> pk=Something' and if pk is None then should it be 'ClassName object 
>>> (unsaved)' or 'ClassName object pk=None' ?
>>>
>>> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:

 Makes sense to me. Maybe still keep the "Transaction object" part, and 
 use None if no pk.

 On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  
 wrote:

> Ticket 27953  is 
> regarding this proposal and the suggestion is about adding "pk" in Model 
> string representation if it exists. 
>
> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>>
>> Hi All,
>>
>> Have you ever quickly set up a model, ran syncdb, and added a few 
>> sample objects in the admin to only see a bunch of "MyModel object"s in 
>> the 
>> changelist? I always forget to add a __unicode__()/__str__() method on 
>> my 
>> models.
>>
>> I ran "git grep -1 __unicode__" on some of my django projects and 
>> noticed a lot of repeated code. In fact, it seems that in about a 
>> _third_ 
>> of all my cases, I'm just returning self.name, or returning self.name 
>> would have been a good default. I looked at a few 3rd party apps for 
>> comparison and found similar results, though not for every app.
>>
>> IMHO, returning self.name (if the field or property exists) is a 
>> sensible default for __unicode__. We can still return "MyModel object" 
>> if 
>> there's no "name" attribute. You'll still end up adding your own 
>> __unicode__ method much of the time, just like you always have.
>>
>> Yes, it's "magic", but we can document it.
>> Yes, it's a little more confusing, but we don't have to explain it 
>> during the tutorial.
>> Yes, it's backwards incompatible, but only in rare cases should it be 
>> a problem.
>> Yes, it could look like any Model without a "name" field is "wrong", 
>> but it's not.
>> Yes, "title" is also very popular, but name is better. :)
>>
>> It has the effect of being a little more friendly in many cases, and 
>> can result in more DRY code.
>>
>> What do your __unicode__/__str__ methods look like? Is this a bad 
>> idea?
>>
>> Thanks,
>> Collin
>>
>> -- 
> You received this message because you are subscribed to the Google 
> Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/c7254a96-7ee3-4262-a90b-83dcfe8fa3d4%40googlegroups.com
>  
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>

 -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/django-developers.
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-developers/1a84fb1c-522f-4bc4-b2

Re: Proposal: make Model __unicode__() default to self.name

2017-04-08 Thread Marco Silva
I have no idea what is the best way, just say that comment. this is the 
original PR
https://github.com/django/django/commit/d2a26c1a90e83dabdf3d67ceec4d2a70fb86

I think you should submit the PR to change the __str__ method, and maybe 
open a new discussion regarding __repr__

sexta-feira, 7 de Abril de 2017 às 15:34:32 UTC+1, Kapil Garg escreveu:
>
> The opened ticket is about Model.__str__ method. Should i open a new 
> ticket for this change ?
> As i see in code, self.__class__ is used in a lot of places but will it 
> effect optimization if we change lookups from self.__class__ to self.cls
>
> Because the methods where class is being used frequently, already store it 
> in local variable and then make references to local variable. 
>
> So should it really be changed ?
>
> On Fri, Apr 7, 2017, 6:52 PM Marco Silva  > wrote:
>
>> I noticed this on the init
>>
>> def __init__(self, *args, **kwargs):
>>  # Alias some things as locals to avoid repeat global lookups
>>  cls = self.__class__
>>
>> maybe you should change it to self.cls??
>> Try to submit a PR to the open ticket.
>>
>> segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>>>
>>> So does this patch seem fine ? 
>>>
>>> diff --git a/django/db/models/base.py b/django/db/models/base.py
>>> index 3c1cb9a..f58e12b 100644
>>> --- a/django/db/models/base.py
>>> +++ b/django/db/models/base.py
>>> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>>>  return '<%s: %s>' % (self.__class__.__name__, u)
>>>  
>>>  def __str__(self):
>>> -return '%s object' % self.__class__.__name__
>>> +return '%s object pk=%s' % (self.__class__.__name__, 
>>> self._get_pk_val())
>>>  
>>>  def __eq__(self, other):
>>>  if not isinstance(other, Model):
>>>
>>>
>>>
>>>
>>> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>>>
>>>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may 
>>>> default to generating a pk before save, so it might just be best to show 
>>>> the actual current pk value
>>>>
>>>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  wrote:
>>>>
>>>>> So what should the final __str__ show: Should it be 'ClassName object 
>>>>> pk=Something' and if pk is None then should it be 'ClassName object 
>>>>> (unsaved)' or 'ClassName object pk=None' ?
>>>>>
>>>>> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:
>>>>>>
>>>>>> Makes sense to me. Maybe still keep the "Transaction object" part, 
>>>>>> and use None if no pk.
>>>>>>
>>>>>> On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  
>>>>>> wrote:
>>>>>>
>>>>>>> Ticket 27953 <https://code.djangoproject.com/ticket/27953> is 
>>>>>>> regarding this proposal and the suggestion is about adding "pk" in 
>>>>>>> Model 
>>>>>>> string representation if it exists. 
>>>>>>>
>>>>>>> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>>>>>>>>
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Have you ever quickly set up a model, ran syncdb, and added a few 
>>>>>>>> sample objects in the admin to only see a bunch of "MyModel object"s 
>>>>>>>> in the 
>>>>>>>> changelist? I always forget to add a __unicode__()/__str__() method on 
>>>>>>>> my 
>>>>>>>> models.
>>>>>>>>
>>>>>>>> I ran "git grep -1 __unicode__" on some of my django projects and 
>>>>>>>> noticed a lot of repeated code. In fact, it seems that in about a 
>>>>>>>> _third_ 
>>>>>>>> of all my cases, I'm just returning self.name, or returning 
>>>>>>>> self.name would have been a good default. I looked at a few 3rd 
>>>>>>>> party apps for comparison and found similar results, though not for 
>>>>>>>> every 
>>>>>>>> app.
>>>>>>>>
>>>>>>>> IM

Re: Proposal: make Model __unicode__() default to self.name

2017-04-08 Thread Marco Silva
just saw that __repr__ is now under discusion here
https://groups.google.com/forum/#!topic/django-developers/UNKFMg6DO5s

sábado, 8 de Abril de 2017 às 17:06:05 UTC+1, Marco Silva escreveu:
>
> I have no idea what is the best way, just say that comment. this is the 
> original PR
>
> https://github.com/django/django/commit/d2a26c1a90e83dabdf3d67ceec4d2a70fb86
>
> I think you should submit the PR to change the __str__ method, and maybe 
> open a new discussion regarding __repr__
>
> sexta-feira, 7 de Abril de 2017 às 15:34:32 UTC+1, Kapil Garg escreveu:
>>
>> The opened ticket is about Model.__str__ method. Should i open a new 
>> ticket for this change ?
>> As i see in code, self.__class__ is used in a lot of places but will it 
>> effect optimization if we change lookups from self.__class__ to self.cls
>>
>> Because the methods where class is being used frequently, already store 
>> it in local variable and then make references to local variable. 
>>
>> So should it really be changed ?
>>
>> On Fri, Apr 7, 2017, 6:52 PM Marco Silva  wrote:
>>
>>> I noticed this on the init
>>>
>>> def __init__(self, *args, **kwargs):
>>>  # Alias some things as locals to avoid repeat global lookups
>>>  cls = self.__class__
>>>
>>> maybe you should change it to self.cls??
>>> Try to submit a PR to the open ticket.
>>>
>>> segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>>>>
>>>> So does this patch seem fine ? 
>>>>
>>>> diff --git a/django/db/models/base.py b/django/db/models/base.py
>>>> index 3c1cb9a..f58e12b 100644
>>>> --- a/django/db/models/base.py
>>>> +++ b/django/db/models/base.py
>>>> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>>>>  return '<%s: %s>' % (self.__class__.__name__, u)
>>>>  
>>>>  def __str__(self):
>>>> -return '%s object' % self.__class__.__name__
>>>> +return '%s object pk=%s' % (self.__class__.__name__, 
>>>> self._get_pk_val())
>>>>  
>>>>  def __eq__(self, other):
>>>>  if not isinstance(other, Model):
>>>>
>>>>
>>>>
>>>>
>>>> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>>>>
>>>>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may 
>>>>> default to generating a pk before save, so it might just be best to show 
>>>>> the actual current pk value
>>>>>
>>>>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  
>>>>> wrote:
>>>>>
>>>>>> So what should the final __str__ show: Should it be 'ClassName object 
>>>>>> pk=Something' and if pk is None then should it be 'ClassName object 
>>>>>> (unsaved)' or 'ClassName object pk=None' ?
>>>>>>
>>>>>> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:
>>>>>>>
>>>>>>> Makes sense to me. Maybe still keep the "Transaction object" part, 
>>>>>>> and use None if no pk.
>>>>>>>
>>>>>>> On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Ticket 27953 <https://code.djangoproject.com/ticket/27953> is 
>>>>>>>> regarding this proposal and the suggestion is about adding "pk" in 
>>>>>>>> Model 
>>>>>>>> string representation if it exists. 
>>>>>>>>
>>>>>>>> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>>>>>>>>>
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Have you ever quickly set up a model, ran syncdb, and added a few 
>>>>>>>>> sample objects in the admin to only see a bunch of "MyModel object"s 
>>>>>>>>> in the 
>>>>>>>>> changelist? I always forget to add a __unicode__()/__str__() method 
>>>>>>>>> on my 
>>>>>>>>> models.
>>>>>>>>>
>>>>>>>>> I ran "git grep -1 __unicode__" on some of my django projects and 
>&

Re: Making __repr__ safe by default

2017-04-08 Thread Marco Silva
going in the python docs we find this

> If at all possible, this should look like a valid Python expression that 
> could be used to recreate an object with the same value (given an 
> appropriate environment).

so maybe something that returned ModelName(field1=value1, field2=value2 
...) where fields with references could return the id of the reference to 
avoid DB lookups 

sábado, 18 de Maio de 2013 às 17:04:48 UTC+1, Patryk Zawadzki escreveu:
>
> As suggested by Marc Tamlyn I am posting this here for discussion: 
>
> https://code.djangoproject.com/ticket/20448 
>
> tl;dr: 
>
> Currently __repr__() calls __unicode__() which can be a very bad 
> thing. You really don't want things like an exception being raised or 
> debugger being used to cause additional side effects (like executing a 
> database query inside __unicode__). 
>
> -- 
> Patryk Zawadzki 
> I solve problems. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1c59d03d-49da-4bfa-836b-31c7d42b63d5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.