Re: Proposal to format Django using black

2019-04-28 Thread Alexander Hill
Hi all,

Could we do the work to make black (or another suitable formatter) apply
only to a range of lines? Prettier has this feature[0], which is used by
precise-commits[1] in the way we'd want to use it for Django. It takes a
range of lines, expands the range to statement boundaries, and formats only
those lines. This feels like a solution nearly everyone could agree on and
would be an asset to the Django and broader Python community.

>From my perspective, the DEP as it stands undervalues blame and Django's
commit history. In PyCharm, the date, message and entire diff of the commit
responsible for every line is immediately available, in the margin of the
editor or with a keyboard shortcut. When it's a trivial to get to, this
context is incredibly useful. Plugins for vim and other tools provide
similar functionality. GitHub is pretty good in this regard too, but it
really shines when integrated into your editor. Like auto-formatting, I
think you don't know how good this is until it's part of your workflow.

To address the counter-arguments: the "view blame prior to this change"
button in GitHub shows you the blame for the entire file before the
specified commit, as opposed to the blame for the file in its current
state, disregarding the specified commit. Not bad, but not quite what you
want. git-hyper-blame has been raised several times, but it's non-standard,
not straightforward to install, and not supported by tooling. A git patch
was mentioned that incorporates hyper-blame's functionality into blame[2],
which looks like it'll be great, but won't help until it's merged,
released, and integrated into tooling.

Django is a mature project, which means that many, many lines have remained
unchanged for years, and will continue to be attributed to the black
mega-commit for many years to come.

I'm firmly -1 to globally applying black to the codebase until it can be
done without breaking blame.

Alex

[0] https://prettier.io/docs/en/options.html#range
[1] https://github.com/nrwl/precise-commits
[2]
https://public-inbox.org/git/CAJDYR9SL9JCJjdARejV=NCf9GYn72=bfszxx84idc416szm...@mail.gmail.com/T/

On Sun, Apr 28, 2019 at 10:51 PM Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> Hello,
>
> Here's my attempt at summarizing the conversation in a DEP:
> https://github.com/django/deps/pull/55.
>
> It's easier to read as a rich diff:
> https://github.com/django/deps/pull/55/files?short_path=95a1a7b#diff-95a1a7b430e2608b84f5c834fd6c258c
>
> Please let me know if I missed or represented unfairly some ideas!
>
> Best regards,
>
> --
> Aymeric.
>
> PS: while I'm eager to listen to feedback and iterate on this draft, I
> would also prefer if this DEP didn't turn into a novel, so let's avoid
> going into every detail and trust the implementation team to do a good job
> — thank you :-)
>
>
> On 13 Apr 2019, at 13:52, Herman S  wrote:
>
> Hi.
>
> I propose that Django starts using 'black' [0] to auto-format all Python
> code.
> For those unfamiliar with 'black' I recommend reading the the projects
> README.
> The short version: it aims to reduce bike-shedding and non value-adding
> discussions; saving time reviewing code; and making the barrier to entry
> lower
> by taking some uncompromissing choices with regards to formatting.  This is
> similar to tools such as 'gofmt' for Go and 'prettier' for Javascript.
>
> Personally I first got involved contributing to Django couple of weeks
> back,
> and from anecdotal experience I can testify to how 'formatting of code'
> creates
> a huge barrier for entry. My PR at the time went multiple times back and
> forth
> tweaking formatting. Before this, I had to research the style used by
> exploring
> the docs at length and reading at least 10-20 different source – and even
> those
> were not always consistent. At the end of the day I felt like almost 50%
> of the
> time I used on the patch was not used on actually solving the issue at
> hand.
> Thinking about code formatting in 2019 is a mental energy better used for
> other
> things, and it feels unnecessary that core developers on Django spend
> their time
> "nit-picking" on these things.
>
> I recently led the efforts to make this change where I work. We have a
> 200K+
> LOC Django code-base with more than 30K commits. Some key take-aways: it
> has
> drastically changed the way we work with code across teams, new engineers
> are
> easier on-boarded, PR are more focused on architectural choices and "naming
> things", existing PRs before migration had surprisingly few conflicts and
> were
> easy to fix, hot code paths are already "blameable" and it's easy to blame
> a
> line of code and go past the "black-commit", and lastly the migration went
> without any issues or down-time.
>
> I had some really fruitful discussions at DjangoCon Europe this week on
> this
> very topic, and it seems we are not alone in these experiences. I would
> love to
> hear from all of you and hope that we can land on something that will
> 

Re: Proposal: relationships based on arbitrary predicates

2018-09-18 Thread Alexander Hill
Hi Silvio,

Thanks for your feedback. David Sanders brought up something similar to
ComputedField on GitHub[0] - a ForwardedField which can traverse
relationships and present a field from a remote instance as if it were a
local field. As long as it can traverse relationships, that use case would
be covered by ComputedField, so that's another tick for that idea.

I think these two features are quite distinct - except that they may both
require a similar change to the ORM to allow traversing relations in the
referred-to fields. One limitation of relativity now is that you can only
form conditions with local fields of your models. I'd like to be able to
traverse relationships in those conditions.

Anybody who's interested - please try out relativity and see if it works
for you. The mptt and treebeard helpers are a good place to start :)

Simon - any further thoughts on this before I start working up a patch?

Thanks,
Alex

[0]
https://github.com/AlexHill/django-relativity/pull/1#issuecomment-418239406

On Mon, 10 Sep 2018 at 4:59 am, Silvio  wrote:

> Alex,
>
> This is a very useful pattern, that like many others, I've also
> implemented in an ad-hoc fashion using a ton of undocumented internal APIs.
> So I fully agree standardizing it would be great. Something similar is:
>
> https://groups.google.com/forum/#!topic/django-developers/ADSuUUuZp3Q
>
> Essentially, I've ended up with the need for:
>
> ComputedField
>
> and
>
> ComputedRelationship
>
> where both have all of the niceties that regular fields and foreign
> relationships have.
>
> So I'd love to see this in Django.
>
> -
> Silvio
>
>
> On Sunday, September 2, 2018 at 10:55:58 PM UTC-4, Alex Hill wrote:
>>
>> Hi Simon,
>>
>> Thanks for looking at this and for providing some context - I had looked
>> at FilteredRelation but I hadn't seen reverse-unique. It makes me more
>> confident that this is a good direction to take. I've reimplemented
>> ReverseUnique using Relationship [0] and the tests pass, with the only code
>> carried over that for discovery of the FK link.
>>
>> > I'm not totally sold on the API but having an analog of what
>> ForeignObject is to ForeignKey for ManyToManyField would definitely be
>> useful.
>>
>> I'm not tied to the API, but I think passing a Q as a predicate makes
>> sense especially given that it's what both FilteredRelation and
>> ReverseUnique do. The core of the idea is that we can express a
>> relationship as a combination of predicate and arity. In practise I don't
>> think this would be used all that much by users directly - more by
>> third-party apps like mptt, and perhaps Django internally.
>>
>> > From what I can see in relativity.fields[0] most of the additional
>> logic revolves around the extra filtering capabilites through Restriction.
>>
>> Yeah that's what it boils down to. We return no columns to join against,
>> and return a compilable Restriction from get_extra_restriction to provide
>> all the ON conditions. The rest of it is making the descriptors, rels,
>> prefetch, etc work.
>>
>> > Do you have an idea of what the fields.related inheritance chain would
>> look like if it was part of core?
>>
>> The least intrusive, and probably a good starting point, would be to
>> introduce Relationship alongside the other relation fields as a standalone
>> feature, modifying the ORM to allow the implementation to be less hacky. It
>> would remain a subclass of ForeignObject (or perhaps RelatedField - I'll
>> give that a try).
>>
>> In the future there's potential for a nice refactor of the ORM to
>> generalise join conditions from key-equality to arbitrary predicates of
>> which key equality is just one case, at which point Relationship could sit
>> comfortably as a base class of all the other relations. The assumption that
>> join==key-equality is pervasive and I think that's an unnecessarily large
>> chunk of work to take on at this point - it would be better to get the
>> feature in, then have a release cycle or so to think about the best way to
>> approach that problem and if we even want to.
>>
>> I would be happy to write up a DEP expanding on an implementation plan
>> and potential future work.
>>
>> Thanks,
>> Alex
>>
>> [0]
>> https://github.com/AlexHill/django-reverse-unique/blob/624c8b19406a40b8e1a2c969c23a6b45bed5a896/reverse_unique/fields.py
>>
>>
>>
>>
>>
>>
>> On Fri, 31 Aug 2018 at 12:12 am, charettes  wrote:
>>
> Hello Alex!
>>>
>>> Thanks for your work on this project, this is definitely something that
>>> I believe would be useful in Django's core based on the number of times I
>>> implemented a filtered queryset getter on Models.
>>>
>>> I'm not totally sold on the API but having an analog of what
>>> ForeignObject is to ForeignKey for ManyToManyField would definitely be
>>> useful.
>>>
>>> From what I can see in relativity.fields[0] most of the additional logic
>>> revolves around the extra filtering capabilites through Restriction.
>>>
>>> Do you have an idea of what the fields.related 

Re: Proposal: relationships based on arbitrary predicates

2018-09-02 Thread Alexander Hill
Hi Simon,

Thanks for looking at this and for providing some context - I had looked at
FilteredRelation but I hadn't seen reverse-unique. It makes me more
confident that this is a good direction to take. I've reimplemented
ReverseUnique using Relationship [0] and the tests pass, with the only code
carried over that for discovery of the FK link.

> I'm not totally sold on the API but having an analog of what
ForeignObject is to ForeignKey for ManyToManyField would definitely be
useful.

I'm not tied to the API, but I think passing a Q as a predicate makes sense
especially given that it's what both FilteredRelation and ReverseUnique do.
The core of the idea is that we can express a relationship as a combination
of predicate and arity. In practise I don't think this would be used all
that much by users directly - more by third-party apps like mptt, and
perhaps Django internally.

> From what I can see in relativity.fields[0] most of the additional logic
revolves around the extra filtering capabilites through Restriction.

Yeah that's what it boils down to. We return no columns to join against,
and return a compilable Restriction from get_extra_restriction to provide
all the ON conditions. The rest of it is making the descriptors, rels,
prefetch, etc work.

> Do you have an idea of what the fields.related inheritance chain would
look like if it was part of core?

The least intrusive, and probably a good starting point, would be to
introduce Relationship alongside the other relation fields as a standalone
feature, modifying the ORM to allow the implementation to be less hacky. It
would remain a subclass of ForeignObject (or perhaps RelatedField - I'll
give that a try).

In the future there's potential for a nice refactor of the ORM to
generalise join conditions from key-equality to arbitrary predicates of
which key equality is just one case, at which point Relationship could sit
comfortably as a base class of all the other relations. The assumption that
join==key-equality is pervasive and I think that's an unnecessarily large
chunk of work to take on at this point - it would be better to get the
feature in, then have a release cycle or so to think about the best way to
approach that problem and if we even want to.

I would be happy to write up a DEP expanding on an implementation plan and
potential future work.

Thanks,
Alex

[0]
https://github.com/AlexHill/django-reverse-unique/blob/624c8b19406a40b8e1a2c969c23a6b45bed5a896/reverse_unique/fields.py






On Fri, 31 Aug 2018 at 12:12 am, charettes  wrote:

> Hello Alex!
>
> Thanks for your work on this project, this is definitely something that I
> believe would be useful in Django's core based on the number of times I
> implemented a filtered queryset getter on Models.
>
> I'm not totally sold on the API but having an analog of what ForeignObject
> is to ForeignKey for ManyToManyField would definitely be useful.
>
> From what I can see in relativity.fields[0] most of the additional logic
> revolves around the extra filtering capabilites through Restriction.
>
> Do you have an idea of what the fields.related inheritance chain would
> look like if it was part of core? I feel like having
> Relation(RelatedField), ForeignObject(Relation), ManyToManyField(Relation)
> and adding the filtering logic to Relation could work but I'd be interested
> to hear what you think here. FWIW Anssi implemented something similar[1]
> for reverse unique relationship before FilteredRelation() was introduced.
>
> In a sense Relation would be form of virtual field like ForeignObject
> since it's not in charge of any database field handling.
>
> Simon
>
> [0]
> https://github.com/AlexHill/django-relativity/blob/3802608c64e86c62ab6268efc37a3f5ca8539221/relativity/fields.py
> [1] https://github.com/akaariai/django-reverse-unique
>
>
>
> Le jeudi 30 août 2018 11:38:16 UTC-4, Alex Hill a écrit :
>>
>> Hi all,
>>
>> I've run into many situations during my time using Django where I've
>> wanted to be able to express relations based on some other criteria than
>> foreign key equality. A few examples:
>> - descendants or children of a node in a tree structure
>> - saved search terms to search results
>> - a model containing a date range to timestamped items falling within
>> that date range
>>
>> Currently to do this kind of thing, you might write a getter which
>> returns a queryset - think for example mptt's get_descendants(). But you
>> don't get any of the nice things a real relation field gives you - you
>> can't use that relationship in filters, you can't select/prefetch_related()
>> or values(), there's no reverse relationship, etc.
>>
>> I've written a Relationship field[0] that lets you define relations in
>> terms of arbitrary Q filters containing objects of a new type, L. An L is
>> like an F, but represents a field on the "local" or "left" side of the
>> relation, where the Q is filtering against the remote "to" side of the
>> relation. For example, in a materialised path tree, this 

Proposal: relationships based on arbitrary predicates

2018-08-30 Thread Alexander Hill
Hi all,

I've run into many situations during my time using Django where I've wanted
to be able to express relations based on some other criteria than foreign
key equality. A few examples:
- descendants or children of a node in a tree structure
- saved search terms to search results
- a model containing a date range to timestamped items falling within that
date range

Currently to do this kind of thing, you might write a getter which returns
a queryset - think for example mptt's get_descendants(). But you don't get
any of the nice things a real relation field gives you - you can't use that
relationship in filters, you can't select/prefetch_related() or values(),
there's no reverse relationship, etc.

I've written a Relationship field[0] that lets you define relations in
terms of arbitrary Q filters containing objects of a new type, L. An L is
like an F, but represents a field on the "local" or "left" side of the
relation, where the Q is filtering against the remote "to" side of the
relation. For example, in a materialised path tree, this is how you might
express descendants:

class Node(models.Model):
path = models.TextField()
descendants = Relationship(
"self",
Q(path__startswith=L('path'), pk__ne=L('pk')),
multiple=True,
reverse_multiple=True,
related_name='ascendants',
)

Now you can use the descendants field like any other many-to-many field in
all the places I mentioned above, but the relationship is based purely on
prefix-matching on the path field. You also get an ascendants field on
Node, which represents the path back to the root and can be used in the
same way.

I think this could make a nice new feature for Django. It would give a
usability boost to anyone using MPTT or treebeard, for example. It works OK
as a third-party library, but the current implementation relies heavily on
undocumented ORM internals, and there are a few features I'd like to
implement that are impractical without making some ORM changes.

Thoughts/feedback/questions welcome!

Thanks,
Alex

[0] https://github.com/alexhill/django-relativity

-- 
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/CA%2BKBOKzXDHJnO09Trkws2MJDu4DnOUsgxgA634xpCAKk1O5xJw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Automatic prefetching in querysets

2017-08-15 Thread Alexander Hill
I think this is an excellent suggestion.

It seems generally accepted in this thread that although there are cases
where this would hurt performance, it would on average solve more problems
than it creates. The debate seems to be more whether or not it's "right"
for the ORM to behave in this magical way. With that in mind...

It won't affect experienced users. They'll read the release notes, see that
this change has been implemented, and either go and delete a bunch of
prefetch_related() calls, grumble a bit and turn auto-prefetch off globally
or just file it away as another fact they know about the Django ORM.

For beginners, this can defer the pain of learning about N+1 problems,
potentially forever depending on the scale of the project. Ultimately
Django's job isn't to teach hard lessons about SQL gotchas, it's to make it
easy to make a nice website. This proposal would reduce both average load
time of Django pages and debugging time, at the cost of the average Django
developer being a little more ignorant about what queries the ORM
generates. I think that's a good trade and in line with the goals of the
project.

Django's ORM isn't SQLAlchemy - it's built on high-level concepts, designed
to be relatively beginner-friendly, and already pretty magical.
select_related() and prefetch_related() are somewhat awkward plugs for a
leaky abstraction, and IMO this is just a better plug.

Alex


On Wed, 16 Aug 2017 at 12:12 Cristiano Coelho 
wrote:

> I would rather have warnings as well, adding more magical behavior is bad
> and might even degrade performance on some cases, automatically selecting a
> bunch of data that "might" be used is bad, and specially considering how
> slow python is, accidentally loading/building 1k+ objects when maybe only
> one of them is used would be as bad as doing 1k+ queries.
>
> If the systems you are building are that large and complicated you can't
> have people with 0 SQL knowledge doing stuff neither! So many things to
> tweak, indexes, data denormalization, proper joins here and there, unique
> constraints, locks and race conditions, someone attempting to code
> something that's not a blog or hello world really needs to know a bit about
> all of that.
>
>
>
> El martes, 15 de agosto de 2017, 6:44:19 (UTC-3), Gordon Wrigley escribió:
>>
>> I'd like to discuss automatic prefetching in querysets. Specifically
>> automatically doing prefetch_related where needed without the user having
>> to request it.
>>
>> For context consider these three snippets using the Question & Choice
>> models from the tutorial
>>  
>> when
>> there are 100 questions each with 5 choices for a total of 500 choices.
>>
>> Default
>> for choice in Choice.objects.all():
>> print(choice.question.question_text, ':', choice.choice_text)
>> 501 db queries, fetches 500 choice rows and 500 question rows from the DB
>>
>> Prefetch_related
>> for choice in Choice.objects.prefetch_related('question'):
>> print(choice.question.question_text, ':', choice.choice_text)
>> 2 db queries, fetches 500 choice rows and 100 question rows from the DB
>>
>> Select_related
>> for choice in Choice.objects.select_related('question'):
>> print(choice.question.question_text, ':', choice.choice_text)
>> 1 db query, fetches 500 choice rows and 500 question rows from the DB
>>
>> I've included select_related for completeness, I'm not going to propose
>> changing anything about it's use. There are places where it is the best
>> choice and in those places it will still be up to the user to request it. I
>> will note that anywhere select_related is optimal prefetch_related is still
>> better than the default and leave it at that.
>>
>> The 'Default' example above is a classic example of the N+1 query
>> problem, a problem that is widespread in Django apps.
>> This pattern of queries is what new users produce because they don't know
>> enough about the database and / or ORM to do otherwise.
>> Experieced users will also often produce this because it's not always
>> obvious what fields will and won't be used and subsequently what should be
>> prefetched.
>> Additionally that list will change over time. A small change to a
>> template to display an extra field can result in a denial of service on
>> your DB due to a missing prefetch.
>> Identifying missing prefetches is fiddly, time consuming and error prone.
>> Tools like django-perf-rec 
>> (which I was involved in creating) and nplusone
>>  exist in part to flag missing
>> prefetches introduced by changed code.
>> Finally libraries like Django Rest Framework and the Admin will also
>> produce queries like this because it's very difficult for them to know what
>> needs prefetching without being explicitly told by an experienced user.
>>
>> As hinted at the top I'd like to propose changing Django so the 

Re: I would like to discuss my proposal for a working way to call .add() on an m2m with through= model

2017-03-21 Thread Alexander Hill
Here's a little bit more historical discussion on the topic:
*https://groups.google.com/d/topic/django-developers/uWe31AjzZX0/discussion
*

On Wed, 22 Mar 2017 at 05:57 Russell Keith-Magee 
wrote:

> On Tue, Mar 21, 2017 at 2:37 PM, Adam Johnson  wrote:
>
> It does seem like a somewhat arbitrary historical restriction. Collin's
> PoC change is surprisingly small and simple.
>
> Seems like it would be fine if Django allowed add() and let any errors
> about missing data bubble-up.
>
>
> As the person who *made* the “somewhat arbitrary historical restriction”…
> :-)
>
> Honestly - the reason it was made was scope creep.
>
> I was trying to land Eric’s work on #6095, which was already moderately
> complex. Nobody disagreed that adding an object to an m2m with a through
> model was a *bad* idea - there were just a few design decisions that had to
> be made. But if we made those decisions, #6095 was going to take *another*
> couple of months to land; the perfect being the enemy of the good, we
> decided to limit scope and land “simple” m2m add, and revisit the issue
> later.
>
> I guess now is “later”… :-)
>
> Yours,
> Russ Magee %-)
>
> --
> 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/CAJxq849m632K%3DaMfXGBtF%3DhMXFS9ujzU6xfUzNxSRkkN_UrkqQ%40mail.gmail.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-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/CA%2BKBOKyNyc1_mcUQiQEiv2FanRLHfU6tR3Sxcv30CcLWgWEqQg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Minor concerns about auth.models.User default settings

2016-10-27 Thread Alexander Hill
Hi David,

Some background reading about the field length:
https://groups.google.com/d/topic/django-developers/v3wYKWWYFT4/discussion
https://code.djangoproject.com/ticket/20846

This contains some discussion as well:
https://groups.google.com/d/topic/django-developers/h98-oEi7z7g/discussion =

And regarding the Unicode username support:
https://code.djangoproject.com/ticket/21379

Django's development process is pretty consistent, so it's generally easy
to find this stuff out by using git blame. See here:
https://github.com/django/django/blame/master/django/contrib/auth/models.py

In the left-hand column, you have the commit responsible for each line,
which has a ticket number. If you go to
http://code.djangoproject.com/ticket/ you'll find what you're
looking for.

Cheers,
Alex

On Fri, 28 Oct 2016 at 10:41 David Tan  wrote:

> In 1.10 auth.models.User.username changed from max_length 30 to 150 and
> defaults to UnicodeValidator.
>
> 1. max_length=150 pushes the username string from 1-byte overhead to
> 4-bytes overhead
>  in
> Postgres.
>
> 2. (Opinionated) ASCIIValidator would've been the more conservative
> default setting while Unicode should've been opt-in.
>
> Respectfully, would someone please explain to me the reasons for these
> decision in 1.10?
>
> --
> 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/a9c6215b-c081-417f-ab9d-c2c174d5d942%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-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/CA%2BKBOKyeKS_rp%3DjrewQhh1eMkdJgosnx63AEwhePe5yZdxxw8Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: AbstractBaseUser.set_unusable_password() why a random string instead of an empty string?

2016-10-19 Thread Alexander Hill
Hi Jon,

Here's the ticket where this was proposed:
https://code.djangoproject.com/ticket/20079

And the commit that implements it:
https://github.com/django/django/commit/aeb1389442d0f9669edf6660b747fd10693b63a7

Cheers,
Alex


On Thu, 20 Oct 2016 at 08:20 Jon Dufresne  wrote:

When set_unusable_password() is called on a user object, the user's
password is set to a random string starting with "!" [0]. The "!" is then
used by is_password_usable() [1] to determine that this password isn't
usable.

My question is, why is a random string used instead of an empty string? An
empty string would appear to make the code both simpler and slightly more
efficient. Is the random string more secure or solving some other issue I'm
not aware of?

I tested this idea and all tests pass:
https://github.com/jdufresne/django/commit/2332194b449fe4a336c8ea515221955ba0ea3aeb

The change is relatively easy as all interactions with unusable passwords
are nicely abstracted by functions

Thoughts on me following through with this code simplification?

[0]
https://github.com/django/django/blob/90c3b11e873a326219a68c14a3b5dca0181e7b3c/django/contrib/auth/hashers.py#L78
[1]
https://github.com/django/django/blob/90c3b11e873a326219a68c14a3b5dca0181e7b3c/django/contrib/auth/hashers.py#L27

-- 
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/CADhq2b7v6BxetkaS6GOW5%3DO1EoXFX9Ke1FWzmCLUs%3DLDjQ5DsA%40mail.gmail.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-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/CA%2BKBOKySg7uPpgYDXJ9aq0gL92-bztoZ5FwKbXyNXPDi4yeqng%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Add an argument to ModelChoiceField for callable function to set label for instance.

2016-09-20 Thread Alexander Hill
I like this!

Having read through the existing ticket and discussion, really the only
reason given is a cultural one: that subclassing is the way this kind of
behaviour "should" be achieved. I disagree – IME, APIs that encourage
parametrising small chunks of behaviour are succinct and flexible, and are
often much more pleasant to deal with than subclassing.

For me there are two issues here. Firstly, the creeping cognitive burden –
another symbol in your IDE, another class to look up, more coupling, more
boilerplate, more tiny choices to make about what to call your class and
where to put it, etc.

The second issue is of readability: if I'm reading a form definition which
includes somebody's BookModelChoiceField, I don't know what that does.
Maybe all it does is override label_from_instance, but I don't know that
until I go look it up. However, as a Django user I do know what
ModelChoiceField does, I know the behaviour of the label_func parameter, so
I can take that all in at a glance and move on.

This all lies on a spectrum in that the more places you use your subclass,
the more justification it has for existing. But when subclassing is the
only solution, you end up defining classes that make only tiny changes to
behaviour and that you instantiate in only one place, and to me that's an
opportunity for improvement in an API.

Alex

P.S.

Another way I have dealt with this issue generically is by overriding
ModelChoiceField._get_choices() to return an overridden ModelChoiceIterator
which yields for each choice not a tuple, but a ModelChoice – a tuple
subclass with an "instance" attribute. It's a little convoluted, but it
means that in your template (and wherever else), you have access to the
actual model instance and can do whatever you like there. If you're not
doing custom things in your template it's not that useful, though.

See gist at
https://gist.github.com/AlexHill/4a5583ab8f983044e2ff04458b52ce4a



On Wed, 21 Sep 2016 at 10:09 Tim Graham  wrote:

> The approach you suggested was suggested in the thread of ticket you
> mentioned:
> https://groups.google.com/d/topic/django-developers/7DDEX73zVrI/discussion
>
> Brian Rosner: "I am a +1 on a configuration parameter since the
> alternative by subclassing is a bit too involved for something fairly
> trivial."
>
> Why was it rejected? What new arguments are you bringing to the table that
> weren't considered before?
>
> You say that subclassing is a burden, but I'd argue that even redefining
> the field on the model form isn't DRY, at which point what about making the
> customization in the form's __init__()?
>
> self.fields['field_name'].label_from_instance = lambda obj: 'new label'
>
> I think the barrier to violate the Zen of Python ("There should be one--
> and preferably only one --obvious way to do it.") is a bit high here, so
> perhaps you could elaborate on how this is causing maintenance problems?
>
>
> On Tuesday, September 20, 2016 at 9:52:55 PM UTC-4, Lawrence Vanderpool
> wrote:
>>
>> Older related ticket: https://code.djangoproject.com/ticket/4620
>>
>> My rough draft of proposed changes:
>> https://gist.github.com/mekhami/24af779f4f491d3c66e6fd607c2121aa/revisions
>>
>> The problem of setting the label for ModelChoiceField is one that comes
>> up on IRC every once in a while. It's a common enough problem that I think
>> the documentation-recommended solution of subclassing ModelChoiceField and
>> setting label_from_instance (
>> https://docs.djangoproject.com/en/1.10/ref/forms/fields/#modelchoicefield
>> ) is overkill, and probably a maintenance problem.
>>
>> ModelChoiceField already implements an optional callable in the
>> `get_limit_choices_to` function. This change would implement a similar API
>> to allow a callable to be passed in to the constructor rather than having
>> to do the subclassing dance.
>>
>> It'd still be backwards compatible, but the documentation should be
>> changed to reflect the "easier" solution of passing in the callable.
>>
>> The old ticket was labelled wontfix, but I think we should take a look
>> again. I'd love for this to be my first contribution to Django! Thanks for
>> any feedback. And if I forgot something or broke a rule for the ML, I
>> apologize in advance @.@
>>
> --
> 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/f79d0b45-5073-4332-80df-0ca21cba1d98%40googlegroups.com
> 
> .

PEP 484 type hinting in Django

2016-08-16 Thread Alexander Hill
Hi all,

I like the plan to include PEP 484 type hinting in Django, outlined in the
PyCharm promotion blog post. [1]

Has this proposal been fleshed out much? Is there any extra information
available anywhere that I've missed? I think this will be great for Django,
and I'd happily contribute to the effort.

Cheers,
Alex

[1]
https://www.djangoproject.com/weblog/2016/jun/30/pycharm-and-django-fundraiser/

-- 
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/CA%2BKBOKzHPhsqeZGF4ebcc2eCuMjpSsN4MfWFAMxzFXjU%3D2-isw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


ModelSignals with abstract senders

2016-05-31 Thread Alexander Hill
Hi all, currently when a model signal receiver is registered with an
abstract model as the sender, we allow it even though the receiver will
never be called. It would be nice if the signal registry wasn't polluted by
bogus entries that will never be used.

A few options:

a) Make passing an abstract model to a ModelSignal a no-op, and document
that behaviour. (And possibly: add a return value to ModelSignal.connect()
like we have for disconnect(), and return False when an abstract model is
passed as a sender)

b) Raise a warning, and make it the user's responsibility to check if a
model is abstract before registering it as a sender.

c) Allow it, and make ModelSignal respect inheritance, so that you can
specify a superclass as a sender and the receiver will be called for all
its subclasses, perhaps controlled by an argument to connect().

Perfectly reasonable code like that in django-model-utils [1] does this, so
I don't think it warrants a warning. My inclination is to go with (a).

Cheers,
Alex

[1]
https://github.com/carljm/django-model-utils/blob/c94b65d05f4acd0437a22ef498a80470922a438a/model_utils/fields.py#L106

-- 
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/CA%2BKBOKzAbyejrwgNrU9r_H3M5B%3D5gJSkxbBr90BxtdUg-91sZA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Lazy operations refactor regression with abstract models #25858

2016-02-15 Thread Alexander Hill
Hi Simon,

Nope, you're not missing anything. I agree with reverting the fix in #25858
and going with approach #2 as outlined in your initial post.

Sorry that wasn't clear - I meant to address my response to the thread as a
whole, in light of that fix being found to cause another regression.

Cheers,
Alex

On Tue, Feb 16, 2016 at 4:42 AM, charettes  wrote:

> Hi Alex, thanks for chiming in.
>
> Just to make sure I understand your second proposed option; I'm having a
> hard
> time understanding how it's different from simply reverting the fix for
> #25858[1].
>
> Are you suggesting that the abstract model's foreign key be resolved or the
> copy.copy()'ied field attached to the concrete model be. If the former is
> ever resolved it will breaks the behavior reported in #26186 as the second
> concrete model subclassing the abstract one will point to the first
> application
> referred model. If only the latter are resolved this will be equivalent to
> reverting the fix for #25858[1].
>
> Am I missing something?
>
> Simon
>
> [1] https://code.djangoproject.com/ticket/25858
> [2] https://code.djangoproject.com/ticket/26186
>
>
> Le mercredi 10 février 2016 22:27:40 UTC-5, Alex Hill a écrit :
>>
>> It looks like we agree that this depending on import order is not on, so
>> we have no choice but to break behaviour in some cases.
>>
>> Option 1 (don't allow relative references) removes support for relative
>> references in abstract models for everyone.
>>
>> Option 2 (resolve references relative to the first concrete inheriting
>> class) doesn't remove anybody's existing behaviour, just formalises it and
>> clarifies how it's specified. The most anybody has to do to keep doing what
>> they're doing is add an app label to a reference.
>>
>> Option 3 (always resolve references relative to the abstract model) makes
>> impossible something that is currently possible: resolving relative
>> references in the concrete class's app.
>>
>> I like option 2. Being able to define a "floating" reference to a
>> to-be-implemented model in a different app is a useful feature in abstract
>> models.
>>
>> If we can't agree on that, I'd go with option 1. I don't really see
>> abstract models as belonging to an app in the same way concrete models do,
>> and that's reflected in Django in various ways. They're not recognised as
>> models by the app registry, fields in their concrete subclasses belong to
>> the inheriting class (unlike in concrete inheritance), we have app label
>> placeholders for related names, etc. I see them more as a blueprint or
>> template than a model. Options 1 and 2 both reinforce that distinction,
>> option 3 muddies it a bit.
>>
>> Cheers,
>> Alex
>>
>>
>> On Thursday, February 11, 2016 at 5:29:52 AM UTC+8, charettes wrote:
>>>
>>> I should have mentioned that this behavior is reproducible since at least
>>> Django 1.6 and has not been introduced by Django 1.8. I wouldn't be
>>> surprised
>>> if it has always been working before the fix was introduced.
>>>
>>> Still, as you mentionned the conditions required to achieve this were
>>> really
>>> convoluted before the lazy operations refactor.
>>>
>>> Simon
>>>
>>> Le mercredi 10 février 2016 16:11:43 UTC-5, Shai Berger a écrit :

 On Tuesday 09 February 2016 23:33:50 charettes wrote:
 > Hi everyone,
 >
 > The chosen fix[1] unfortunately introduced a new regression[2].
 >
 > It looks like the behavior described in the previous ticket was
 possible
 > with
 > Django 1.8 under certain circumstances[3] where the abstract models
 defined
 > in a foreign application were derived as concrete models in another
 > applications
 > before the abstract models relations are resolved (if they are ever
 > resolved):
 >

 The explanation is complex enough, the case where it works edgy enough,
 that
 I'd be content to call this a bug in 1.8. I'm pretty sure the specifics
 of this
 resolution are not documented, and my sense from reading the ticket is
 that if
 you'd try to document the behavior you'll reach the conclusion that it
 probably isn't intentional.

 My 2 cents,
 Shai.

 >
 > [1]
 https://github.com/django/django/commit/bc7d201bdbaeac14a49f51a9ef292d6
 > [2] https://code.djangoproject.com/ticket/26186
 > [3] https://code.djangoproject.com/ticket/26186#comment:8
 > [4] https://code.djangoproject.com/ticket/24215

>>> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/jRut8aYEet0/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group 

Re: Refactor year, month, day lookups?

2015-03-25 Thread Alexander Hill
In the dicts produced by a values query like that, what key would the
result be found under? The full lookup string i.e. 'pub_date__month'?

On Thu, Mar 26, 2015 at 2:36 AM, Anssi Kääriäinen 
wrote:

> There remains a bit of work to do to make transforms usable everywhere in
> the ORM. The example should work with just
> .values('pub_date__month').annotate(...)
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers  (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/WYWrQkBJ2hs/unsubscribe
> .
> To unsubscribe from this group and all its topics, 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 http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/1b62a33e-f0b6-4153-bbc3-2e48ebdb7ac7%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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CA%2BKBOKx%2BOYZu%3D9OSKRFMgzgHvQV%2B54yrhKC86jF%2BT9%2B8Mi2e%2Bw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Adding an option on db_index to not create text/varchar_pattern_ops

2015-03-20 Thread Alexander Hill
Hi Marc,

Great, that's very interesting reading! A lot of similarities to what I had
in mind and a couple of differences.

As I imagine the API, Indexes would be backend-specific rather than
generic. Field would have a get_db_indexes() method which took a connection
as its first argument like get_db_prep_value(), db_type(), etc. Every
backend would implement a common interface to meet the most common needs,
and from there its Indexes would be free to diverge.

Are the various DBMS's indexes really similar enough that the differences
can be abstracted over in an elegant and useful way? What does such
abstraction offer?

Take the operator class example that prompted this thread: how would
TextField specify that it needed a text_pattern_ops index in PostgreSQL?

Cheers,
Alex

On Fri, Mar 20, 2015 at 2:50 PM, Marc Tamlyn  wrote:

> I have some plans (see https://github.com/django/deps/pull/6) to rework
> more widely how indexes are managed and allow finer control over them. It
> pretty much includes all the things mentioned by Alex. I'm intending on
> doing some work on it over the next few weeks.
>
> Marc
>
> On 20 March 2015 at 02:27, Alex Hill  wrote:
>
>> I agree that this is a problem, and I'd like to see more control over
>> indexes generally in Django.
>>
>> However, at first glance I suspect that fixing only your problem would
>> mean adding special-case gunk to the Field class in addition to the
>> Postgres schema editor where it currently lives[1]. It feels wrong to
>> expand this special-casing when other types could benefit from having
>> customisable indexes. For instance, the db_index parameter is useless for
>> column types that don't work with a btree index, like jsonb and tsvector.
>>
>> To fix your immediate problem, can't you avoid creating the extra indexes
>> by just setting db_index=False, and then use django-json-dbindex to define
>> the indexes you DO want?
>>
>> I would like to see an index API that allowed Field subclasses to
>> specify, per-backend, the indexes that should be created if db_index is
>> True. Those defaults should be overridable by the user, perhaps by passing
>> a sequence of some Index type as the db_index parameter instead of simply
>> True or False.
>>
>> Not all indexes relate to a single field, so it would make sense to also
>> be able to register indexes in a model's Meta as well. The Index type
>> should be able to use Expressions to create functional and multi-column
>> indexes.
>>
>> So, that's my wishlist. All that said if you can come up with a patch
>> that fixes your problem in a sane way, great! :)
>>
>> Cheers,
>> Alex
>>
>> [1]
>> https://github.com/django/django/blob/35b3158d52a5fe51d3b52aec1109a30a73c5abe9/django/db/backends/postgresql_psycopg2/schema.py#L22
>>
>> On Friday, March 20, 2015 at 3:44:58 AM UTC+8, rodolphe.q...@novapost.fr
>> wrote:
>>>
>>> As proposed by timgraham in https://code.djangoproject.com/ticket/24507
>>> I open the discussion here.
>>>
>>> When adding db_index=True on a CharField Django automatically create 2
>>> indexes on some backends as PostgreSQL. But in usage the second index is
>>> not always necessary if you never use contains() or similar queries. As you
>>> can see here I extracted indexes usages statistics from one of our
>>> production server.
>>>
>>> The index *foo_bar_email_from_create_like* is never use even if
>>> *foo_bar_email_from_create* is, and if we look on our queries this is
>>> totally logic and regular. And it's the same for *foo_bar_tech_id* and
>>> *foo_bar_user_type*, and it's the same on the other table.
>>>
>>> indexrelname  |  idx_scan  | idx_tup_read |
>>> idx_tup_fetch
>>> --++
>>> --+--
>>> foo_bar_address_like  |  0 |0 |
>>> 0
>>> foo_bar_current_profile_id|   1846 |  617 |
>>>   236
>>> foo_bar_date_delete   |  0 |0 |
>>> 0
>>> foo_bar_email_from_create |  31209 |90886 |
>>> 21903
>>> foo_bar_email_from_create_like|  0 |0 |
>>> 0
>>> foo_bar_entity_id |   8026 |28957 |
>>>14
>>> foo_bar_pkey  | 1258565593 |   1418841848 |
>>> 1194873240
>>> foo_bar_site_id   |4495829 |  51000840065 |
>>>  3564
>>> foo_bar_tech_id   |   25045160 | 28233693 |
>>>  25087324
>>> foo_bar_tech_id_like  |  0 |0 |
>>> 0
>>> foo_bar_user_type |  21428 |263769329 |
>>> 216686751
>>> foo_bar_user_type_like|  0 |0 |
>>> 0
>>> foo_bar_uuid_like |  0 |0 |
>>> 0
>>> foo_bar_uuid_uniq |   13134415 | 13157636 |

Abstract models and the app registry

2015-02-11 Thread Alexander Hill
Hi all,

I'm looking for some background about abstract models, specifically why
they aren't registered with the app registry as normal models are.

For some context, I'm working on a ticket [0] to refactor Django's lazy
model operations (used e.g. to resolve string references in related
fields). To acquire an actual model class from an "app_label.ModelName"
string, we use Apps.get_registered_model. However, as far as that method is
concerned, abstract models don't exist, so operations that wait on them are
never run.

It also means abstract models clutter the pending operations dict, making
it impossible to provide a good warning when there's a typo in a string
model reference, for example.

Fortunately this doesn't matter too much in practise, because concrete
subclasses work fine despite related fields not being hooked up right on an
abstract parent.

[0] https://code.djangoproject.com/ticket/24215

Cheers,
Alex

-- 
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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CA%2BKBOKz5V3TcqpphKxUx3gLfWvk9-G61%2B4AgCQHrQ6mZy6DocA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Displaying (some) M2M fields with through models specified

2014-05-25 Thread Alexander Hill
Hi all,

Currently, only M2M fields without custom intermediary models can use the
normal Django M2M form machinery. If you define your own intermediary
models, you need to use inlines instead.

I would like to allow fields with custom through models to use the regular
M2M field/widget automatically, if the through model meets the requirement
that all its extra fields either have defaults or are nullable. I'll refer
to these as "auto-compatible" models.

Making this work right now is as easy as adding "auto_created = True" to
the intermediary model's Meta. If your model is not auto-compatible, you'll
get a error when you try to add to the relation.

I would like to add official support for this. My suggestion would be:

1. Add a model Meta attribute that better describes these models, e.g.
"simple_m2m". Auto-created intermediary models should have this set to True.
2. Wherever auto_created appears in a condition, decide for each whether or
not auto-compatible models should also pass the test - if so, replaced with
a check to "simple_m2m".
3. Add code to validate that user-created models with simple_m2m True
actually do meet the above requirements.

As a note to step 2, if there is no test of auto_created that
auto-compatible models shouldn't also pass, auto_created could simply be
renamed. I doubt that though.

Thoughts?

Alex

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CA%2BKBOKykoNVUZLR_DGdYx%2Bfd87ZcpGWku25ZR8japaPG2fdReg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.