Re: Issue with get_FOO_display not working in Django admin

2023-04-05 Thread 'Ibrahim Abou Elenein' via Django developers (Contributions to Django itself)
Isn't this some sort of duplication? why not just use it instead of writing 
its logic again?
On Wednesday, April 5, 2023 at 7:12:48 AM UTC+2 David Sanders wrote:

> Hi Ibrahim,
>
> get_FOO_display() isn't intended to be overridden like that, it's just a 
> convenience method for use in templates/whatever that refers to the 
> underlying flatchoices.
>
> For clarity, please see the documentation: 
> https://docs.djangoproject.com/en/4.2/ref/models/instances/#django.db.models.Model.get_FOO_display
>
> Kind regards,
> David
>
> On Wed, 5 Apr 2023 at 09:10, 'Ibrahim Abou Elenein' via Django developers 
> (Contributions to Django itself)  wrote:
>
>> Dear All,
>>
>> I am writing to report an issue I encountered while working with Django 
>> admin. I had a model with a field that uses Choices as follows:
>>
>> ```
>> status = FSMField(default=STATUSES.PENDING, choices=STATUSES, 
>> protected=True)
>> ```
>> I overrode the get_status_display method, but to my surprise, it did not 
>> have any effect in the Django admin.
>>
>> Upon investigating Django's code, I found the following method in 
>> contrib.admin
>>
>> ```
>> def display_for_field(value, field, empty_value_display):
>> from django.contrib.admin.templatetags.admin_list import _boolean_icon
>>
>> if getattr(field, "flatchoices", None):
>> return dict(field.flatchoices).get(value, empty_value_display)
>> ```
>> I noticed that this method uses flatchoices to get the display value for 
>> fields with choices. However, it does not take into account any custom 
>> display methods that may have been defined for the field.
>>
>> To resolve this issue, I modified the code to use get_FOO_display instead 
>> of flatchoices as follows:
>>
>> ```
>> def display_for_field(value, field, empty_value_display, model=None):
>> from django.contrib.admin.templatetags.admin_list import _boolean_icon
>>
>> if getattr(field, "flatchoices", None):
>> if model:
>> return getattr(model, "get_%s_display" % field.name)()
>> return dict(field.flatchoices).get(value, empty_value_display)
>> ```
>> This modification allowed my custom display method to work as expected in 
>> the Django admin.
>>
>> However, I am curious to know why Django's code behaves this way and how 
>> I can make use of this behavior in my application.
>>
>> Thank you for your attention to this matter.
>>
>> Sincerely,
>> Ibrahim.
>>
>> -- 
>> 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 view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/4f432f39-b959-422e-b062-5db54722b18en%40googlegroups.com
>>  
>> 
>> .
>>
>

-- 
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/f60ef476-412e-4739-b43e-15e181d7a986n%40googlegroups.com.


Re: Issue with get_FOO_display not working in Django admin

2023-04-05 Thread David Sanders
At this point I'll let others chime in with their opinion on whether this
is something that needs to change because:

   1. I rarely use admin
   2. I've never really had the need to override a choice's display over
   those supplied via `choices`

:)

On Wed, 5 Apr 2023 at 19:05, 'Ibrahim Abou Elenein' via Django developers
(Contributions to Django itself)  wrote:

> Isn't this some sort of duplication? why not just use it instead of
> writing its logic again?
>
>

-- 
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/CADyZw-7r%3D%3Dwybp8XWp7tEdAOYWuhKucXGE_Kjq0twTy%3DN8KWPg%40mail.gmail.com.


Proposal: Check constraints at the model field level

2023-04-05 Thread David Sanders
Hi folks,

We've had check constraints for a while now and they're awesome.

I'd like to propose an alternative way to declare check constraints: at the
field level. This sounds like it's duplicating the feature but there are
some advantages that make this distinct from declaring at the model-level
from the meta:

   - Colocality: the check rules are close to the field they're concerned
   with
   - Reusability: Allows for bundling with custom field types
   - Good for checks concerned only with the field its declared on, for
   multiple fields recommended to continue to use Meta. (Kind of analogous to
   unique=True vs UniqueConstraint)

For example:

class Product(Model):
price = DecimalField(..., check=Q(price__gte=0))
...
other fields
...


class Meta:
constraints = [
... declare constraints here that are concerned with multiple
fields...
]


For more complex fields you can then bundle the check for reusability:

class PriceField(DecimalField):
def contribute_to_class(self, ...):
super().contribute_to_class(...)
self.check = Q(**{f'{self.name}__gte': 0})


Some other points:

   - To be consistent with model-level check constraints they'd also need
   to participate in validate_constraints().
   - (Small)PositiveIntegerField already has its own implementation of a
   check constraint, enforcing values >- 0 via the private db_check() method.
   I think this is an example of how bundling checks can be useful.
   - I won't go into implementation alternatives but making use of this
   existing db_check() method is one possibility. How participation in
   validation would work would still need to be decided upon.
   - See this Stupid Django Trick
   

   for some experimentation with this idea.


Cheers,
David

-- 
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/CADyZw-6Odv0dpo-En3Jm2NqPhBtK7ebaGKBi4OROe1n%2BvHEE-A%40mail.gmail.com.


Re: Proposal: Check constraints at the model field level

2023-04-05 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
I agree this feature would be useful, at least to allow bundling check
constraints with custom field classes. As you point out the
PositiveIntegerField classes do this within Django, and doubtless many
custom fields have used the db_check() method.

The only thing I'm not a fan of in your proposal is repeating the field
name within the check expression, like "price" in:

price = DecimalField(..., check=Q(price__gte=0))

This seems repetitive at least, and would increase error refactoring. Also
Django field classes typically don't “know” their name until the Model meta
class logic runs, and contribute_to_class() is called. Requiring the name
within check() would mean it would need to accept any name and validate it
later.

Perhaps we could support only a special name instead, like “self” or the
shorter “f”?

price = DecimalField(..., check=Q(f__gte=0))

On Wed, Apr 5, 2023 at 7:18 PM David Sanders 
wrote:

> Hi folks,
>
> We've had check constraints for a while now and they're awesome.
>
> I'd like to propose an alternative way to declare check constraints: at
> the field level. This sounds like it's duplicating the feature but there
> are some advantages that make this distinct from declaring at the
> model-level from the meta:
>
>- Colocality: the check rules are close to the field they're concerned
>with
>- Reusability: Allows for bundling with custom field types
>- Good for checks concerned only with the field its declared on, for
>multiple fields recommended to continue to use Meta. (Kind of analogous to
>unique=True vs UniqueConstraint)
>
> For example:
>
> class Product(Model):
> price = DecimalField(..., check=Q(price__gte=0))
> ...
> other fields
> ...
>
>
> class Meta:
> constraints = [
> ... declare constraints here that are concerned with multiple
> fields...
> ]
>
>
> For more complex fields you can then bundle the check for reusability:
>
> class PriceField(DecimalField):
> def contribute_to_class(self, ...):
> super().contribute_to_class(...)
> self.check = Q(**{f'{self.name}__gte': 0})
>
>
> Some other points:
>
>- To be consistent with model-level check constraints they'd also need
>to participate in validate_constraints().
>- (Small)PositiveIntegerField already has its own implementation of a
>check constraint, enforcing values >- 0 via the private db_check() method.
>I think this is an example of how bundling checks can be useful.
>- I won't go into implementation alternatives but making use of this
>existing db_check() method is one possibility. How participation in
>validation would work would still need to be decided upon.
>- See this Stupid Django Trick
>
> 
>for some experimentation with this idea.
>
>
> Cheers,
> David
>
> --
> 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/CADyZw-6Odv0dpo-En3Jm2NqPhBtK7ebaGKBi4OROe1n%2BvHEE-A%40mail.gmail.com
> 
> .
>

-- 
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/CAMyDDM3Tyj%3Djfo9yFiUythymjP2WHKGi6cJY%3DOKk1_XkggiMtQ%40mail.gmail.com.


Stalebot on djangoproject.com issues

2023-04-05 Thread Tim Graham
Hello,

In October 2022, a stalebot was activated for djangoproject.com issues:

https://github.com/django/djangoproject.com/issues/1219
https://github.com/django/djangoproject.com/pull/1220

It comments on an issue if there's no activity in the last six months: "This 
issue has been automatically marked as stale because it has not had recent 
activity. It will be closed if no further activity occurs. Thank you for 
your contributions."

The bot closes the issue if there's no activity in the next seven days.

I didn't see much discussion among the djangproject.com team outside of the 
issue and PR,  but the rationale from the issue is this: "Idea copied from 
DRF PR #8423  - Add 
a StaleBot, configured with the lowest possible run limit. The intent here 
is to help us sweep through the issue and pull request backlog, and review 
what does and does not need to remain open at this point in time."

Here is what I said at the time: "I think this is a lame way to handle old 
issues. The result seems to be Carlton triaging all issues that the bot 
comments on. You could have asked him to do that without a bot adding 
noise. Should a useless "issue still valid" comment be required every 180 
days? Why not have a human triage each issue now that more people are 
maintaining this site? Using a bot comes off to me as passive aggressive. 
Why try to automatically discard years of issues (even if minor)? It's not 
like the passage of time or lack of activity means the problem went away. A 
responsible reporter will look through existing issues so they don't report 
a duplicate and not necessarily leave a comment like "issue still valid." 
If we close the issue automatically, what did that accomplish? I would 
think triaging issues would be a good way for new team members to develop 
their understanding of the site. If you're feeling unknowledgable about an 
old issue, feel free to ping me for advice. I hope you might reconsider the 
usefulness of the bot."

It's six months later, and I'm now having to again comment on inactive 
issues "stalebot, please don't close." to keep valid issues open.

Paolo Melchiorre (author of this initiative) says, "activating the stalebot 
last year allowed us to close many old issues. I think an issue opened 7 
years ago should be closed if no one takes care of it despite two reminders 
in the last year. As the removal of the stalebot is not only up to me, I 
think it is worth discussing it in a separate issue or on the developer 
mailing list."

I remain of the opinion that continued attempts to automatically abandon 
valid issues are not helpful and do not reflect project maintenance best 
practices. I would like to hear your thoughts on the matter. Thanks!

-- 
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/84d1ee45-401b-4a52-8243-1a40fcf32955n%40googlegroups.com.


Re: Proposal: Check constraints at the model field level

2023-04-05 Thread Mariusz Felisiak
Hi,

This proposal is not really nice from a maintenance point of view as we 
will end with the same complicated situation we currently have with 
uniqueness checks or indexes i.e. many ways to define the same:

- Field.unique/index
- Meta.unique_together/index_together
- Meta.constraints/indexes

It's especially error-prone in migrations and different database behavior 
on fields already covered by the same constraints/indexes. I'm pretty sure 
that we've introduced Meta.contraints/indexes to avoid this happening in 
the future, and we are rather leaning to leave only 
Meta.constraints/indexes and remove other options in the future. Not 
creating a new one.

Initial -1 from me.

Best,
Mariusz

-- 
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/954af838-2176-4877-b4ac-70525cddcbf5n%40googlegroups.com.


Re: Proposal: Check constraints at the model field level

2023-04-05 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
Mariusz, I agree with the burden, but it should be noted that SQL has both
CHECK on the field and table level, and CheckConstraint only defines
table-level constraints. This is not true for unique constraints or indices.

Also, what do you think of a way for custom field classes to add
constraints, at least? db_check() is somewhat limiting given it must return
raw SQL, plus it's undocumented.

On Thu, Apr 6, 2023 at 5:11 AM Mariusz Felisiak 
wrote:

> Hi,
>
> This proposal is not really nice from a maintenance point of view as we
> will end with the same complicated situation we currently have with
> uniqueness checks or indexes i.e. many ways to define the same:
>
> - Field.unique/index
> - Meta.unique_together/index_together
> - Meta.constraints/indexes
>
> It's especially error-prone in migrations and different database behavior
> on fields already covered by the same constraints/indexes. I'm pretty sure
> that we've introduced Meta.contraints/indexes to avoid this happening in
> the future, and we are rather leaning to leave only
> Meta.constraints/indexes and remove other options in the future. Not
> creating a new one.
>
> Initial -1 from me.
>
> Best,
> Mariusz
>
> --
> 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/954af838-2176-4877-b4ac-70525cddcbf5n%40googlegroups.com
> 
> .
>

-- 
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/CAMyDDM0QonqK_7ibWKFf7d8eX7QxzSD7JNRKsJn3o4y%2BYgHVnQ%40mail.gmail.com.