Re: Proposal to format Django using black

2019-04-19 Thread Jacob Rief
I share the opinion of Mariusz Felisiak, Luke Plant and Claude Paroz, and 
believe that it is a bad idea to do this in an automatic way without the 
possibility to interfere manually.
- Jacob

-- 
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/3195031d-e5e7-4a60-aa79-249010577a80%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to format Django using black

2019-04-19 Thread Mariusz Felisiak
I'm against adopting black to Django. I don't like style proposed by black 
(some of rules make code less readable) and in the same time I really enjoy 
Django's style, but that's of course my subjective opinion.

I don't think that our code style is any barrier for newcomers. Just like 
Tim I make a minor stylistic edits before merging patches (if necessary) 
and it is not really a big issue for me. I double-check all lines of 
patches so minor edits don't increase the time that I spend on PR. I want 
to clarify that adopting black will not immediately solve any tickets 
because we've never blocked any patch due to stylistic nitpicks. I also 
don't believe that it will increase the number of contributors, if I would 
like to contribute to any package it wouldn't matter to me.

IMO we should stay with isort, flake8 and hanging indentation (we don't 
have many extra rules). Of course I'm open for discussion about relaxing 
our code policy.

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 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/1fe71fd2-5c3c-4ad6-8e3b-f172f07aacaa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to format Django using black

2019-04-19 Thread Claude Paroz
I must admit I'm rather pleased by Luke's proposal. Let's keep our current 
isort/flake8 checks and relax other policies (considering the committer can 
always minor edit patches, as Tim did in the past).

I'm always a bit suspicious of tools that decide the formatting for me.

Claude

-- 
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/dae569ab-371d-40d5-bf2e-2cd4f4d38ed6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to format Django using black

2019-04-19 Thread Luke Plant

  
  
Hi Tim,
I didn't mean to imply the original decisions were foolish, in
  their context. Reading it again, my language in the email was too
  strong - it was directed at myself as much as anyone else, because
  I'm also the kind of person who loves getting everything
  consistent, often too much, and that wasn't clear. What I meant to
  say was that if it turns out that our guidelines are becoming a
  problem, we should drop them, or change how we regard them (e.g.
  as guidelines for those who want to go the extra mile, not as
  rules that will frustrate contributors).

Your work on the Django code base has been tireless and
  fantastic, and along with the other fellows I don't know what we'd
  do without you, sorry for not making that clear with my comments.

Thanks,
 Luke


On 19/04/2019 15:51, Tim Graham wrote:


  
  I'm sorry for establishing some (foolish?)
guidelines that discouraged some people from contributing. I
didn't establish those guidelines merely to be pedantic, but
perhaps I'm too much of a perfectionist at times.

After GitHub allowed mergers to amend pull requests, I often
made cosmetic adjustments myself to eliminate the trivial back
and forth that some have lamented. I never blocked a merge
merely for trivial cosmetic reasons.

On Friday, April 19, 2019 at 3:25:24 AM UTC-4, Luke Plant wrote:

  
Hi all,
An alternative solution to the problem of nit-picky,
  formatting related PR reviews is simple: let's not do
that. 

We already have an automated code formatting enforcer,
  flake8. Let's simply drop the requirement on fixing
  anything that flake8 doesn't pick up. A committer can fix
  up style issues if they want to, but shouldn't make anyone
  else do it. This would mean most of the stuff on our coding style page
  should just be delete, or at least not enforced - by which
  I mean almost anything that can't be enforced by a tool
  (such as isort, flake8, editors via .editorconfig etc.),
  and has no non-local effects. (So consistent naming of
  classes/functions *should* be enforced, because that
  affects other people's ability to use the code).  Large
  parts of that page are just duplicating of flake8/isort
  rules anyway. Honestly, does it kill us if someone writes
  "we" in a code comment? And black couldn't help with some
  of these things anyway. Let's just stop being code review
  jerks.

I'm kind of ambivalent on black itself. Certainly there
  are cases where it makes code less readable (a significant
  sin in my book) e.g. lists that are better displayed
  vertically, as mentioned already, and there are cases
  where it makes your diff larger than it needs to be (e.g.
  when it decides a list is now too long and needs to be
  re-formatted vertically). If we adopt black we'd have to
  live with those annoyances. Alternatively, we can live
  with the annoyance that code formatting is not perfectly
  consistent and we accept less than 'perfect' PR. But we
  should just live with those things:


    A foolish consistency
  is the hobgoblin
of little minds


And if our consistency requirements are causing problems
  for people attempting to contribute, they are foolish and
  should be dropped.
My 2 ¢.
Luke


On 18/04/2019 16:03, Jani Tiainen wrote:


  Well let me add my two cents here since I
was also in the group in DCEU that talked about the
usage of black.


Personally I don't like to contribute to Django.
  And this is why:


Day one: I'll make the fix/patch and create PR
Day two (or four or five depending how busy
  reviewers are): I missed a comma or some minor indent
  is wrong
Day three: I fix styles
Day four: PR is accepted.


So whole round trip took about a five days (give a
  take few usually depending how busy reviewers are).


That gives me a feeling that I'm really wasting my
  time and since I can't get all the

Re: Proposal to format Django using black

2019-04-19 Thread Tobias McNulty
FWIW, I like that we have high standards for concise and readable comments
with proper grammar, especially because I myself often need a little extra
push in that department. And I'm not easily convinced by the arguments that
sound a whole lot like "I've written the code and don't have time to clean
this up." In the thick of the moment it's easy to assume a solution will be
obvious to others reading the code, when in reality taking a few extra
minutes to clearly write down *why *something is the way it is (not to
mention making the code itself readable in the first place) will save
potentially hours of someone else's time later on. This is precisely what
code reviews are for; to make sure we're handing down well-written,
described, and documented code for those who come after us (including
perhaps ourselves, a year or two down the road). The benefit to us as
contributors is that it makes us stronger developers, and I think that
might get forgotten sometimes.

I've continued to follow along in this thread and remain fairly ambivalent
about Black. I see how it will simplify many things; on the other hand, I
like my multi-line lists formatted my way. (Not poking fun, I really do.)
That said, if Black will truly remove some significant hurdles, let's do
it, but we should remain clear-headed about the fact that it's not going to
remove from the equation what some may feel is nitpicking and others would
argue is a necessary part of maintaining our high standards.

Herman, if you haven't been scared away from this just yet, I might suggest
trying to summarize the discussion and your recommendation/proposal given
the reactions so far (could be a DEP  but
doesn't have to be, IMO). I do feel there's a pragmatic solution in here
somewhere.

Best to all,
Tobias



*Tobias McNulty*Chief Executive Officer

tob...@caktusgroup.com
www.caktusgroup.com

On Fri, Apr 19, 2019 at 8:52 AM Tim Graham  wrote:

> I'm sorry for establishing some (foolish?) guidelines that discouraged
> some people from contributing. I didn't establish those guidelines merely
> to be pedantic, but perhaps I'm too much of a perfectionist at times.
>
> After GitHub allowed mergers to amend pull requests, I often made cosmetic
> adjustments myself to eliminate the trivial back and forth that some have
> lamented. I never blocked a merge merely for trivial cosmetic reasons.
>
> On Friday, April 19, 2019 at 3:25:24 AM UTC-4, Luke Plant wrote:
>>
>> Hi all,
>>
>> An alternative solution to the problem of nit-picky, formatting related
>> PR reviews is simple: *let's not do that*.
>>
>> We already have an automated code formatting enforcer, flake8. Let's
>> simply drop the requirement on fixing anything that flake8 doesn't pick up.
>> A committer can fix up style issues if they want to, but shouldn't make
>> anyone else do it. This would mean most of the stuff on our coding style
>> page
>> 
>> should just be delete, or at least not enforced - by which I mean almost
>> anything that can't be enforced by a tool (such as isort, flake8, editors
>> via .editorconfig etc.), and has no non-local effects. (So consistent
>> naming of classes/functions *should* be enforced, because that affects
>> other people's ability to use the code).  Large parts of that page are just
>> duplicating of flake8/isort rules anyway. Honestly, does it kill us if
>> someone writes "we" in a code comment? And black couldn't help with some of
>> these things anyway. Let's just stop being code review jerks.
>>
>> I'm kind of ambivalent on black itself. Certainly there are cases where
>> it makes code less readable (a significant sin in my book) e.g. lists that
>> are better displayed vertically, as mentioned already, and there are cases
>> where it makes your diff larger than it needs to be (e.g. when it decides a
>> list is now too long and needs to be re-formatted vertically). If we adopt
>> black we'd have to live with those annoyances. Alternatively, we can live
>> with the annoyance that code formatting is not perfectly consistent and we
>> accept less than 'perfect' PR. But we should just live with those things:
>>
>>
>> *A* *foolish* *consistency* *is* *the* *hobgoblin** of little minds*
>>
>>
>> And if our consistency requirements are causing problems for people
>> attempting to contribute, they are foolish and should be dropped.
>>
>> My 2 ¢.
>>
>> Luke
>>
>>
>> On 18/04/2019 16:03, Jani Tiainen wrote:
>>
>> Well let me add my two cents here since I was also in the group in DCEU
>> that talked about the usage of black.
>>
>> Personally I don't like to contribute to Django. And this is why:
>>
>> Day one: I'll make the fix/patch and create PR
>> Day two (or four or five depending how busy reviewers are): I missed a
>> comma or some minor indent is wrong
>> Day three: I fix styles
>> Day four: PR is accepted.
>>
>> So whole round trip took about a five days 

Re: Proposal to format Django using black

2019-04-19 Thread Tim Graham
I'm sorry for establishing some (foolish?) guidelines that discouraged some 
people from contributing. I didn't establish those guidelines merely to be 
pedantic, but perhaps I'm too much of a perfectionist at times.

After GitHub allowed mergers to amend pull requests, I often made cosmetic 
adjustments myself to eliminate the trivial back and forth that some have 
lamented. I never blocked a merge merely for trivial cosmetic reasons.

On Friday, April 19, 2019 at 3:25:24 AM UTC-4, Luke Plant wrote:
>
> Hi all,
>
> An alternative solution to the problem of nit-picky, formatting related PR 
> reviews is simple: *let's not do that*. 
>
> We already have an automated code formatting enforcer, flake8. Let's 
> simply drop the requirement on fixing anything that flake8 doesn't pick up. 
> A committer can fix up style issues if they want to, but shouldn't make 
> anyone else do it. This would mean most of the stuff on our coding style 
> page 
> 
>  
> should just be delete, or at least not enforced - by which I mean almost 
> anything that can't be enforced by a tool (such as isort, flake8, editors 
> via .editorconfig etc.), and has no non-local effects. (So consistent 
> naming of classes/functions *should* be enforced, because that affects 
> other people's ability to use the code).  Large parts of that page are just 
> duplicating of flake8/isort rules anyway. Honestly, does it kill us if 
> someone writes "we" in a code comment? And black couldn't help with some of 
> these things anyway. Let's just stop being code review jerks.
>
> I'm kind of ambivalent on black itself. Certainly there are cases where it 
> makes code less readable (a significant sin in my book) e.g. lists that are 
> better displayed vertically, as mentioned already, and there are cases 
> where it makes your diff larger than it needs to be (e.g. when it decides a 
> list is now too long and needs to be re-formatted vertically). If we adopt 
> black we'd have to live with those annoyances. Alternatively, we can live 
> with the annoyance that code formatting is not perfectly consistent and we 
> accept less than 'perfect' PR. But we should just live with those things:
>
>
> *A* *foolish* *consistency* *is* *the* *hobgoblin** of little minds*
>
>
> And if our consistency requirements are causing problems for people 
> attempting to contribute, they are foolish and should be dropped.
>
> My 2 ¢.
>
> Luke
>
>
> On 18/04/2019 16:03, Jani Tiainen wrote:
>
> Well let me add my two cents here since I was also in the group in DCEU 
> that talked about the usage of black. 
>
> Personally I don't like to contribute to Django. And this is why:
>
> Day one: I'll make the fix/patch and create PR
> Day two (or four or five depending how busy reviewers are): I missed a 
> comma or some minor indent is wrong
> Day three: I fix styles
> Day four: PR is accepted.
>
> So whole round trip took about a five days (give a take few usually 
> depending how busy reviewers are).
>
> That gives me a feeling that I'm really wasting my time and since I can't 
> get all the small bits and pieces exactly as Django wants in correct place.
>
> And that's because we have slightly different rules at the work. And some 
> other projects do have different rules.
>
> So it would be great if some of this pain could be relieved with a tool. 
> In my short experience with black (I've been using it for work projects) it 
> does a pretty decent job.
>
> Like others have said black does some decisions I don't agree with. But I 
> don't have to. Black does it for a "greater good". And after a while black 
> actually vanishes from the flow. 
>
> On Sat, Apr 13, 2019 at 6:35 PM 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

Re: Allowing model field choices to receive a list of strings instead of a list of string tuples

2019-04-19 Thread René Fleschenberg
Hi

> Could we allow lists to be passed to choices ?
> `|choices=['potato', 'carrot', 'turnip']|`

I do not think that this justifies a change in Django. You can do this,
if you like:


def to_choices(values):
return [(x, x) for x in values]

myfield = CharField(
choices=to_choices([
'potato',
'carrot',
'turnip',
])
)


A list of strings might be less cognitive load for those developers who
never need to change (or translate!) the display names, but for all
others, it will be more confusing.

-- 
René Fleschenberg

-- 
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/331699be-426a-3ad9-5422-ee21f41bf98b%40fleschenberg.net.
For more options, visit https://groups.google.com/d/optout.


Allowing model field choices to receive a list of strings instead of a list of string tuples

2019-04-19 Thread Barnaby
Hello,

I remember that, as a beginner Django developer, I stumbled upon the 
`choices` attribute of `models.CharField`.

Could we allow lists to be passed to choices ?
`choices=['potato', 'carrot', 'turnip']`

Which would be the same as this :
`choices=[('potato', 'potato'), ('carrot', 'potato'), ('turnip', 'potato')]`

I understand that storing long strings in the database is not ideal. 
However, it feels like premature optimization that isn't very relevant to 
new Django users. Choices are needed very often in any kind of basic CRUD 
app, and this is something new users will often encounter quite early in 
their Django learning. Allowing a simple list input would reduce the 
cognitive load and make Django easier to learn.

Do you think this makes sense ?

-- 
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/53f30e25-2b05-4573-91a7-a3be013b3286%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Why does ModelForm do validation and not Model

2019-04-19 Thread René Fleschenberg
Hi

On 4/18/19 2:07 PM, Václav Řehák wrote:
>  As a veteran Django user, I am quite used to it but as I work on
> financial project (with strong requirements on data consistency) with a
> team of senior developers kind of new to Django I face a lot of
> confusion about why does Django let us save invalid data (actually last
> week I spent almost 3 days on fixes caused forgotten calls to full_clean
> and on data migration to clean up the mess). If it was possible, e.g. in
> settings, to force model validation in save(), it would help us a lot.
You can make an abstract model with an overriden save() that calls
full_clean() and inherit all your models from it. However, remember that
this does not cover all database writes. For example, update() on a
queryset does not call save(). For this reason, I don't think it is a
good idea. It just gives developers a false sense of security. Instead,
make sure to validate all input at the application boundary (e.g. using
forms).

If data integrity is really important, also consider using
database-level constraints. This can be done using raw SQL from a
migration. In Django 2.2, there also is
https://docs.djangoproject.com/en/2.2/ref/models/constraints/.


-- 
René Fleschenberg

-- 
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/8ce3915a-de41-8fee-c36e-078d003b1677%40fleschenberg.net.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to format Django using black

2019-04-19 Thread Luke Plant

  
  
Hi all,
An alternative solution to the problem of nit-picky, formatting
  related PR reviews is simple: let's not do that. 

We already have an automated code formatting enforcer, flake8.
  Let's simply drop the requirement on fixing anything that flake8
  doesn't pick up. A committer can fix up style issues if they want
  to, but shouldn't make anyone else do it. This would mean most of
  the stuff on our coding
style page should just be delete, or at least not enforced -
  by which I mean almost anything that can't be enforced by a tool
  (such as isort, flake8, editors via .editorconfig etc.), and has
  no non-local effects. (So consistent naming of classes/functions
  *should* be enforced, because that affects other people's ability
  to use the code).  Large parts of that page are just duplicating
  of flake8/isort rules anyway. Honestly, does it kill us if someone
  writes "we" in a code comment? And black couldn't help with some
  of these things anyway. Let's just stop being code review jerks.

I'm kind of ambivalent on black itself. Certainly there are cases
  where it makes code less readable (a significant sin in my book)
  e.g. lists that are better displayed vertically, as mentioned
  already, and there are cases where it makes your diff larger than
  it needs to be (e.g. when it decides a list is now too long and
  needs to be re-formatted vertically). If we adopt black we'd have
  to live with those annoyances. Alternatively, we can live with the
  annoyance that code formatting is not perfectly consistent and we
  accept less than 'perfect' PR. But we should just live with those
  things:


    A foolish consistency
  is the hobgoblin of
little minds


And if our consistency requirements are causing problems for
  people attempting to contribute, they are foolish and should be
  dropped.
My 2 ¢.
Luke


On 18/04/2019 16:03, Jani Tiainen
  wrote:


  
  Well let me add my two cents here since I was also
in the group in DCEU that talked about the usage of black.


Personally I don't like to contribute to Django. And this
  is why:


Day one: I'll make the fix/patch and create PR
Day two (or four or five depending how busy reviewers are):
  I missed a comma or some minor indent is wrong
Day three: I fix styles
Day four: PR is accepted.


So whole round trip took about a five days (give a take few
  usually depending how busy reviewers are).


That gives me a feeling that I'm really wasting my time and
  since I can't get all the small bits and pieces exactly as
  Django wants in correct place.


And that's because we have slightly different rules at the
  work. And some other projects do have different rules.


So it would be great if some of this pain could be relieved
  with a tool. In my short experience with black (I've been
  using it for work projects) it does a pretty decent job.


Like others have said black does some decisions I don't
  agree with. But I don't have to. Black does it for a "greater
  good". And after a while black actually vanishes from the
  flow. 
  
  
  
On Sat, Apr 13, 2019 at 6:35
  PM 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 a