Mailman 3 Conditional dict declarations - Python-ideas

newer
PEP-603: Adding a frozenmap type...

Conditional dict declarations

older
Automatic translation of Python to...

rls＠hwyl.org

4 Sep 2019 4 Sep '19

10:25 p.m.

Hi list I have a proposal for a minor (improvement?) to dict declarations. I find a common requirement I have is to include one or more entries in a dict only if some condition is met. Currently, the usual way of doing that is to add one or more if statements. Simple (but hopefully relatable-to-real-world-code) example. Here we're constructing a query for a database based on the arguments passed in. In some cases we don't want to include some fields in the search at all depending on what the values passed in are. ### def construct_query(user_id, max_results=10000, active=None, deleted=None): query = { 'user_id': user_id } if max_results > 0: query['max_results'] = max_results if active is not None: query['active'] = active if deleted is not None: query['deleted'] = deleted return query ### This is OK-ish, but seems unnecessarily verbose, and it gets worse if you have more conditional entries. In addition, you're separating the dict setup code into two or more places so it is slower to grasp mentally. This is a fairly common (tedious) pattern in my code, but I feel it could be avoided. In my view it would be better if it was possible to conditionally add the entry in the dict declaration. My proposal is that there should be a special value defined that results in a dict entry not being added to the dict if either the key or the value equals this value. There may be several ways to achieve this, but here's one way it could work. ### def construct_query(user_id, max_results=10000, active=None, deleted=False): return { 'user_id': user_id, 'max_results': max_results or dict.skip, 'active': active if active is not None else dict.skip, 'deleted': deleted if deleted is not None else dict.skip } ### ..."skip" being a unique object defined in and accessible from dict (there may be better places to make it available). The dict insertion code then simply ignores those entries when creating the dict. I think this would be relatively easy to achieve. No significant modification to the language, just a minor modification to the dict code (as far as I know).

Show replies by date

Paul Moore

5 Sep 5 Sep

12:03 p.m.

On Thu, 5 Sep 2019 at 11:19, wrote:

...

I find a common requirement I have is to include one or more entries in a dict only if some condition is met. Currently, the usual way of doing that is to add one or more if statements.

Simple (but hopefully relatable-to-real-world-code) example. [...] This is a fairly common (tedious) pattern in my code, but I feel it could be avoided. In my view it would be better if it was possible to conditionally add the entry in the dict declaration.

My proposal is that there should be a special value defined that results in a dict entry not being added to the dict if either the key or the value equals this value. There may be several ways to achieve this, but here's one way it could work.

This is pretty easy to write as a helper function:

...

...
...
def dct(**kw): ... return {k:v for k, v in kw.items() if v is not dct.skip} ... dct.skip = object()

dct(a=12, b=None, c=dct.skip, d="Hello", e=dct.skip) {'a': 12, 'b': None, 'd': 'Hello'}

Given this, I'm not sure it's a sufficiently useful improvement to be worth adding to the builtin dictionary (although thanks for the suggestion, I wouldn't have ever thought about writing a helper to avoid the boilerplate without this posting to prompt me!) Paul

Richard Musil

12:38 p.m.

I was thinking more in this direction: d = dict() d[action in not None and 'action'] = action d[minval > 0 and 'minval'] = minval ... del d[False] There are few things I do not particularly like about this (assigning to dict, even when value is discarded later, or necessity of the del d[False]), but I guess it is as expressive as it could be and I could imagine using it in my app easily. If needed one can even define another key instead of False.

brian.skinn＠gmail.com

1:18 p.m.

I thought of something similar, but where the dict-literal construction is desired:

...

...
...
foo = True bar = False baz = False d = { ... 'foo' if foo else None: 1, ... 'bar' if bar else None: 2, ... 'baz' if baz else None: 3, ... } d {'foo': 1, None: 3} d.pop(None) 3 d {'foo': 1}

The natural helper function for the "'foo' if foo else None" snip would then be something like:

...

...
...
def key_flag(key, flag, default=None): ... return key if flag else default ... dd = { ... key_flag('foo', foo): 1, ... key_flag('bar', bar): 2, ... key_flag('baz', baz): 3, ... } dd {'foo': 1, None: 3} dd.pop(None) 3 dd {'foo': 1}

-Brian

brian.skinn＠gmail.com

1:21 p.m.

Oooookie, looks like my code got munched in the web view -- how do I make it not do that?

Sebastian Kreft

2:46 p.m.

How is this different to the discussion https://mail.python.org/archives/list/python-ideas@python.org/thread/MFSL3U6... ? On Thu, Sep 5, 2019 at 9:24 AM wrote:

...

Oooookie, looks like my code got munched in the web view -- how do I make it not do that? _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/JNUMEI... Code of Conduct: http://python.org/psf/codeofconduct/

-- Sebastian Kreft

Andrew Barnert

5:59 p.m.

...

On Sep 5, 2019, at 07:46, Sebastian Kreft wrote:

How is this different to the discussion https://mail.python.org/archives/list/python-ideas@python.org/thread/MFSL3U6... ?

Well, that discussion started with an unworkable proposal, and generated a variety of different alternatives, some of which were workable, but all of which were buried deep in the discussion of the unworkable idea. Polishing up one of those alternatives and extracting it out into its own new thread doesn’t seem like a bad thing to do. A link back to the old thread might have been nice, but now you’ve fixed that. :)

Joao S. O. Bueno

6 Sep 6 Sep

3:36 a.m.

Also, if one have in mind that dict addition with the `+` operator had been recelently approved, as of Python 3.9 it will be ok to write: ``` return ({ 'user_id': user_id,} + ({'max_results': max_results} if max_results else {}) + ({'active': active} if active is not None else {}) + ({'deleted': deleted} if deleted is not None else {}) ) And actually, on a second though, this is already valid as of current Python - and is potentially more readable than a magic "dict.skip" singleton. ``` return { 'user_id': user_id, **({'max_results': max_results} if max_results else {}), **({'active': active} if active is not None else {}), **({'deleted': deleted} if deleted is not None else {}) } ``` On Thu, 5 Sep 2019 at 10:22, wrote:

...

I thought of something similar, but where the dict-literal construction is desired:

...
...
...
foo = True bar = False baz = False d = { ... 'foo' if foo else None: 1, ... 'bar' if bar else None: 2, ... 'baz' if baz else None: 3, ... } d {'foo': 1, None: 3} d.pop(None) 3 d {'foo': 1}

The natural helper function for the "'foo' if foo else None" snip would then be something like:

...
...
...
def key_flag(key, flag, default=None): ... return key if flag else default ... dd = { ... key_flag('foo', foo): 1, ... key_flag('bar', bar): 2, ... key_flag('baz', baz): 3, ... } dd {'foo': 1, None: 3} dd.pop(None) 3 dd {'foo': 1}

-Brian _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/SZZVOF... Code of Conduct: http://python.org/psf/codeofconduct/

Guido van Rossum

4:53 a.m.

On Thu, Sep 5, 2019 at 8:37 PM Joao S. O. Bueno wrote:

...

Also, if one have in mind that dict addition with the `+` operator had been recelently approved

Where did you hear this? I am not aware that this was even made into a PEP let alone that such a PEP was approved. -- --Guido van Rossum (python.org/~guido) *Pronouns: he/him/his **(why is my pronoun here?)* http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...

Joao S. O. Bueno

7 Sep 7 Sep

3:35 a.m.

On Fri, 6 Sep 2019 at 01:54, Guido van Rossum wrote:

...

On Thu, Sep 5, 2019 at 8:37 PM Joao S. O. Bueno wrote:

...
Also, if one have in mind that dict addition with the `+` operator had been recelently approved

Where did you hear this? I am not aware that this was even made into a PEP let alone that such a PEP was approved.

Well - the idea showed up here, and got an immediate "that is nice, let's do it" from you, followed by approval from a lot more people, and very little controverse. So, it is pretty much "approved", even if not yet detailed in an approved PEP. And 3.9 still sounds quite realistic for that specific feature, unless no one actually writes the PEP. But my bad, I thought the full approval process (PEP included) for that had been carried on already. Nonetheless, the mapping expansion syntax do allow for the OP's needs, and is even a bit shorter than the version using "+".

...

-- --Guido van Rossum (python.org/~guido) *Pronouns: he/him/his **(why is my pronoun here?)* http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...

Steven D'Aprano

4:17 a.m.

On Sat, Sep 07, 2019 at 12:35:09AM -0300, Joao S. O. Bueno wrote:

...

Well - the idea showed up here, and got an immediate "that is nice, let's do it" from you, followed by approval from a lot more people, and very little controverse. So, it is pretty much "approved", even if not yet detailed in an approved PEP.

As the PEP author, I don't consider it even close to approved. I'm still slowly working my way through the next draft of the PEP. There are lots of objections and unresolved questions: - is this feature necessary? - if so, should it be an operator, a dict method or a function? - if an operator, should it be spelled ``+`` or ``|`` or something else? - what precise behaviour should it have? At the time I wrote the first draft, there was no consensus that the feature was necessary, but most people who thought it would be useful agreed that ``+`` was the right operator. Since then, further discussion seemed to shift closer to consensus that the feature might be useful, but away from using ``+``. -- Steven

Joao S. O. Bueno

1:38 p.m.

On Sat, 7 Sep 2019 at 01:24, Steven D'Aprano wrote:

...

On Sat, Sep 07, 2019 at 12:35:09AM -0300, Joao S. O. Bueno wrote:

...
Well - the idea showed up here, and got an immediate "that is nice, let's do it" from you, followed by approval from a lot more people, and very little controverse. So, it is pretty much "approved", even if not yet detailed in an approved PEP.

As the PEP author, I don't consider it even close to approved. I'm still slowly working my way through the next draft of the PEP. There are lots of objections and unresolved questions:

- is this feature necessary?

- if so, should it be an operator, a dict method or a function?

- if an operator, should it be spelled ``+`` or ``|`` or something else?

- what precise behaviour should it have?

At the time I wrote the first draft, there was no consensus that the feature was necessary, but most people who thought it would be useful agreed that ``+`` was the right operator. Since then, further discussion seemed to shift closer to consensus that the feature might be useful, but away from using ``+``. ]

Thank you for the feedback. I will follow the PEP evolution more close now. And indeed, as we can see, even in a case like this in the thread, where the "+" could have been useful (1) the OP did not think of using it at all, coming up with a different approach, and (2) the exiting star-map expansion can do what "+" would do in this case. These would point to that maybe it is not "necessary" at all. Although I still think it would be a useful feature, and, on that, maybe keeping the consistence with the "|" already in use by 'set' would be better.

...

-- Steven _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/OHKUFD... Code of Conduct: http://python.org/psf/codeofconduct/

Andrew Barnert

5 Sep 5 Sep

3:51 p.m.

On Sep 4, 2019, at 15:25, rls@hwyl.org wrote:

...

..."skip" being a unique object defined in and accessible from dict (there may be better places to make it available). The dict insertion code then simply ignores those entries when creating the dict.

I’m a bit confused about exactly when this does and doesn’t come into play. You say here “the dict insertion code” does it. Would they mean d[key]=d.skip doesn’t insert anything? If so, then how could I put skip in a dict at all? If there’s already a d[key] does that get left alone, or deleted? Wouldn’t this even affect dict inserts that happen behind the scenes, like any time you tried to assign dict.skip to a global variable or a class or instance attribute, or pass it as a keyword argument? If an implementation doesn’t use dict.insert to build dict literals, does that mean it doesn’t work in dict literals on that implementation, even though that was the whole motivation for the feature? Maybe instead of starting with how it could be implemented, start with where you ideally do and don’t want it to take effect, and then we can try to work out how it could be implemented to do that, and how to specify it to require such an implementation. * Obviously you want it in non-comprehension dict displays, because that’s your example. * What about comprehensions? * Constructor calls with the keyword-arg API like dict(a=1, b=dict.skip, **d2}? * Constructor calls from iterables of pairs, like dict((1, 2), (3, dict.skip))? * Subscription assignments? * Calls to insert? * Calls to update? * Capturing ** keyword args? * Assignments to globals, attributes, etc. when those use a dict under the covers (as they usually do!? * Assignments to such things when they don’t use a dict (as with a slots class instance)? Also, what should happen with this case: flag0, flag1 = True, False d = {'k': 0 if flag0 else dict.skip, 'k': 1 if flag1 else dict.skip} Do we keep the last value for 'k' as usual, and because that’s dict.skip, therefore d['k'] ends up as a KeyError? Or do we keep the last non-skip value and therefore d['k'] ends up as 0? What about this: class D(dict): skip = 0 d = D() d.skip = 1 d[0] = 0 d[1] = 1 d[dict.skip] = dict.skip Do we skip the one that uses d.skip, the one that uses type(d).skip (like a special method lookup), or the one that uses dict.skip?

Brandt Bucher

4:22 p.m.

Also worth noting is that dict.skip would live in dict.__dict__... which is a dict. Brandt

...

On Sep 5, 2019, at 08:51, Andrew Barnert via Python-ideas wrote:

...
On Sep 4, 2019, at 15:25, rls@hwyl.org wrote:

..."skip" being a unique object defined in and accessible from dict (there may be better places to make it available). The dict insertion code then simply ignores those entries when creating the dict.

I’m a bit confused about exactly when this does and doesn’t come into play.

You say here “the dict insertion code” does it. Would they mean d[key]=d.skip doesn’t insert anything? If so, then how could I put skip in a dict at all? If there’s already a d[key] does that get left alone, or deleted? Wouldn’t this even affect dict inserts that happen behind the scenes, like any time you tried to assign dict.skip to a global variable or a class or instance attribute, or pass it as a keyword argument? If an implementation doesn’t use dict.insert to build dict literals, does that mean it doesn’t work in dict literals on that implementation, even though that was the whole motivation for the feature?

Maybe instead of starting with how it could be implemented, start with where you ideally do and don’t want it to take effect, and then we can try to work out how it could be implemented to do that, and how to specify it to require such an implementation.

* Obviously you want it in non-comprehension dict displays, because that’s your example. * What about comprehensions? * Constructor calls with the keyword-arg API like dict(a=1, b=dict.skip, **d2}? * Constructor calls from iterables of pairs, like dict((1, 2), (3, dict.skip))? * Subscription assignments? * Calls to insert? * Calls to update? * Capturing ** keyword args? * Assignments to globals, attributes, etc. when those use a dict under the covers (as they usually do!? * Assignments to such things when they don’t use a dict (as with a slots class instance)?

Also, what should happen with this case:

flag0, flag1 = True, False d = {'k': 0 if flag0 else dict.skip, 'k': 1 if flag1 else dict.skip}

Do we keep the last value for 'k' as usual, and because that’s dict.skip, therefore d['k'] ends up as a KeyError? Or do we keep the last non-skip value and therefore d['k'] ends up as 0?

What about this:

class D(dict): skip = 0 d = D() d.skip = 1 d[0] = 0 d[1] = 1 d[dict.skip] = dict.skip

Do we skip the one that uses d.skip, the one that uses type(d).skip (like a special method lookup), or the one that uses dict.skip?

_______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/GH3KEN... Code of Conduct: http://python.org/psf/codeofconduct/

Rich Smith

6 Sep 6 Sep

6:21 p.m.

On Thu, 5 Sep 2019 at 16:51, Andrew Barnert wrote:

...

You say here “the dict insertion code” does it. Would they mean d[key]=d.skip doesn’t insert anything?

Yes. If so, then how could I put skip in a dict at all? The intention is that you couldn't. This could raise some implementation issues!

...

If there’s already a d[key] does that get left alone, or deleted?

Left alone.

...

Wouldn’t this even affect dict inserts that happen behind the scenes, like any time you tried to assign dict.skip to a global variable or a class or instance attribute, or pass it as a keyword argument? If an implementation doesn’t use dict.insert to build dict literals, does that mean it doesn’t work in dict literals on that implementation, even though that was the whole motivation for the feature?

Maybe instead of starting with how it could be implemented, start with where you ideally do and don’t want it to take effect, and then we can try to work out how it could be implemented to do that, and how to specify it to require such an implementation.

This was what I was attempting to do. I wasn't suggesting the way I wrote was the way it should be implemented, just a sketch of how it might look.

...

* Obviously you want it in non-comprehension dict displays, because that’s your example. * What about comprehensions?

Yes.

...

* Constructor calls with the keyword-arg API like dict(a=1, b=dict.skip, **d2}?

I'd expect the b value to not be assigned.

...

* Constructor calls from iterables of pairs, like dict((1, 2), (3, dict.skip))?

I'd expect the 3 key to be unassigned (assuming an extra pair of brackets).

...

* Subscription assignments?

I'm sorry, I'm not sure what you're referring to here. Do mean as in x[dict.skip] = y or x[y] = dict.skip? If so, I'd expect both operations to be, well, skipped.

...

* Calls to insert? * Calls to update?

Yes. Inserts to remain unassigned, updates to keep the original value. The idea is to skip the operation.

...

* Capturing ** keyword args?

Yes.

...

* Assignments to globals, attributes, etc. when those use a dict under the covers (as they usually do!?

I'm not sure... setting an attribute/global to "dict.skip" might not have any effect. I don't think the user could complain about surprise for doing something like this.

...

* Assignments to such things when they don’t use a dict (as with a slots class instance)?

Also, what should happen with this case:

flag0, flag1 = True, False d = {'k': 0 if flag0 else dict.skip, 'k': 1 if flag1 else dict.skip}

Do we keep the last value for 'k' as usual, and because that’s dict.skip, therefore d['k'] ends up as a KeyError? Or do we keep the last non-skip value and therefore d['k'] ends up as 0?

I'd expect the assignments to happen in order, so d['k'] would be set to 0 and then the second assignment to be ignored.

...

What about this:

class D(dict): skip = 0 d = D() d.skip = 1 d[0] = 0 d[1] = 1 d[dict.skip] = dict.skip

Do we skip the one that uses d.skip, the one that uses type(d).skip (like a special method lookup), or the one that uses dict.skip?

This seems like a fairly pathological case, and I'm not sure it's really in the spirit of your request to focus on the use rather than the implementation! But I'd expect the key value of dict.skip to cause the entry to be skipped. Thanks for coming up with so many angles on this; it's interesting exercise to consider how many things could be affected by this. Writing a ConditionalDict type in a similar spirit would be a lot simpler, though less convenient to use. I will experiment. Thanks Rich

Andrew Barnert

8:09 p.m.

On Friday, September 6, 2019, 11:22:08 AM PDT, Rich Smith wrote:

...

On Thu, 5 Sep 2019 at 16:51, Andrew Barnert wrote:

...
You say here “the dict insertion code” does it. Would they mean d[key]=d.skip doesn’t insert anything? > Yes. OK, then I go from +0 on your proposal to -1. Constructing dicts with optionally-included values is a bit overly-verbose, no matter which of the dict-construction mechanisms you use, and it comes up pretty often, so sugar to make it less verbose but just as clear... I don't see a burning need for it, but I've got no objections. But a magic value that can never be stored in any dict? That sounds nice at first, but I think once you think it through, it would be a nightmare. Especially when you consider how fundamental dicts are to all of Python. Globals is a dict. Locals is a dict in some contexts, not others (and the set of contexts may even be different in different Python implementations). Instance attributes are a dict on most classes, but not all. Class attributes are a dict. The namespace passed to the metaclass in a class definition is a dict. The values gathered for `**kw` are a dict. And so on. The rest of your answers are consistent with what you'd expect from this "can never go into a dict" design, so I don't have any additional problem with any of them; it's just the fundamental idea that I disagree with. :) But one more comment: If so, then how could I put skip in a dict at all? The intention is that you couldn't. This could raise some implementation issues! That's putting it mildly. Forget about figuring out how to make `dict` work (which requires understanding the bytecodes and C API functions and so on). Imagine that you're maintaining one of the popular pure-Python alternative `collections.abc.Mapping` types on PyPI, say, `MySortedDict`. Python 3.9 adds `dict.skip`, and your users quite reasonably want you to add the same feature to your type: `MySortedDict.skip is dict.skip`, and passing that as a key or value to insert/update/construction/etc. skips the same way it does for dict. Does the fact that you can't store `dict.skip` in a global, or an attribute of your class, or pass it as a keyword argument, etc. get in the way? Writing a ConditionalDict type in a similar spirit would be a lot simpler, though less> convenient to use. I will experiment. If you're interested, it's pretty easy to write an import hook that replaces all dict displays with calls to your type's constructor. That's obviously not something you'd want to use in real-life code, but I've found it useful for playing with ideas like this before. I think one of the repos with a name ending in "hack" at abarnert - Overview will get you 90% of the way there, but I don't remember exactly what's up there.

| | | | | | | | | | | abarnert - Overview abarnert has 102 repositories available. Follow their code on GitHub. | | |

Rob Cliffe

7 Sep 7 Sep

8:38 a.m.

On 04/09/2019 23:25:32, rls@hwyl.org wrote:

...

Hi list

I have a proposal for a minor (improvement?) to dict declarations.

I find a common requirement I have is to include one or more entries in a dict only if some condition is met. Currently, the usual way of doing that is to add one or more if statements.

Simple (but hopefully relatable-to-real-world-code) example. Here we're constructing a query for a database based on the arguments passed in. In some cases we don't want to include some fields in the search at all depending on what the values passed in are.

###

def construct_query(user_id, max_results=10000, active=None, deleted=None):

query = { 'user_id': user_id }

if max_results > 0: query['max_results'] = max_results

if active is not None: query['active'] = active

if deleted is not None: query['deleted'] = deleted

return query

### A chance for me to bang the drum on one of my pet themes: Sometimes the readability of code is improved by breaking the sacred taboo of 1 statement per line, if it allows similar constructs to be vertically aligned:

if max_results > 0 : query['max_results'] = max_results if active is not None : query['active'] = active if deleted is not None: query['deleted'] = deleted If this comes out ragged in your browser, the colons and equal signs are meant to be vertically aligned. Rob Cliffe

Chris Angelico

5:59 p.m.

On Sat, Sep 7, 2019 at 11:27 PM Rob Cliffe via Python-ideas wrote:

...

A chance for me to bang the drum on one of my pet themes: Sometimes the readability of code is improved by breaking the sacred taboo of 1 statement per line, if it allows similar constructs to be vertically aligned:

if max_results > 0 : query['max_results'] = max_results if active is not None : query['active'] = active if deleted is not None: query['deleted'] = deleted

If this comes out ragged in your browser, the colons and equal signs are meant to be vertically aligned.

I'm not bothered by putting an if and a simple statement together, but I am definitely bothered by the need to vertically align them, and especially by the triple repeated name. Not a fan of this style - looks like a bug magnet and maintenance burden. ChrisA

Rob Cliffe

8 Sep 8 Sep

6:09 p.m.

On 07/09/2019 18:59:49, Chris Angelico wrote:

...

On Sat, Sep 7, 2019 at 11:27 PM Rob Cliffe via Python-ideas wrote:

...
A chance for me to bang the drum on one of my pet themes: Sometimes the readability of code is improved by breaking the sacred taboo of 1 statement per line, if it allows similar constructs to be vertically aligned:

if max_results > 0 : query['max_results'] = max_results if active is not None : query['active'] = active if deleted is not None: query['deleted'] = deleted

If this comes out ragged in your browser, the colons and equal signs are meant to be vertically aligned. I'm not bothered by putting an if and a simple statement together, but I am definitely bothered by the need to vertically align them, and especially by the triple repeated name. Not a fan of this style - looks like a bug magnet and maintenance burden.

ChrisA The OP's code:

if max_results > 0: query['max_results'] = max_results if active is not None: query['active'] = active if deleted is not None: query['deleted'] = deleted is perfectly reasonable and has the same triple repeated name. Would you rewrite it differently, and if so, how? I'm just saying that aligning similar elements and bringing them together, without intervening lines, makes it easier to see the code structure and spot typos. Not to mention using less vertical screen space. I don't understand why you think it is a bug magnet and maintenance burden. I think it is *easier* to maintain: in which of the following is it easier to spot the mistake? # Version 1: if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted # Version 2: if max_results > 0: querydata['max_results'] = max_results if active is not None: querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted Data point: I have used this style repeatedly in my code (only where appropriate, of course) and find it helpful. Rob Cliffe

Chris Angelico

6:34 p.m.

On Mon, Sep 9, 2019 at 4:13 AM Rob Cliffe via Python-ideas wrote:

...

On 07/09/2019 18:59:49, Chris Angelico wrote:

...
On Sat, Sep 7, 2019 at 11:27 PM Rob Cliffe via Python-ideas wrote:

...
A chance for me to bang the drum on one of my pet themes: Sometimes the readability of code is improved by breaking the sacred taboo of 1 statement per line, if it allows similar constructs to be vertically aligned:

if max_results > 0 : query['max_results'] = max_results if active is not None : query['active'] = active if deleted is not None: query['deleted'] = deleted

If this comes out ragged in your browser, the colons and equal signs are meant to be vertically aligned. I'm not bothered by putting an if and a simple statement together, but I am definitely bothered by the need to vertically align them, and especially by the triple repeated name. Not a fan of this style - looks like a bug magnet and maintenance burden.

ChrisA The OP's code:

if max_results > 0: query['max_results'] = max_results if active is not None: query['active'] = active if deleted is not None: query['deleted'] = deleted

is perfectly reasonable and has the same triple repeated name. Would you rewrite it differently, and if so, how? I'm just saying that aligning similar elements and bringing them together, without intervening lines, makes it easier to see the code structure and spot typos. Not to mention using less vertical screen space. I don't understand why you think it is a bug magnet and maintenance burden. I think it is *easier* to maintain: in which of the following is it easier to spot the mistake?

# Version 1: if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

# Version 2: if max_results > 0: querydata['max_results'] = max_results if active is not None: querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

Data point: I have used this style repeatedly in my code (only where appropriate, of course) and find it helpful.

Now add a fourth one, for result_count. What does your diff look like? Do you have to edit every other line just to realign it? That's the maintenance burden - fiddling around with formatting. Honestly, I couldn't see the mistake in _either_ version until I looked pretty closely, upon which it was equally visible in both. If I were using this in a full app, it would be something a linter could easily spot, again regardless of the alignment. You're right that the triple repeated name is not caused by your style. However, your suggestion also doesn't solve it. What I'd probably want to do, here, is build the dictionary with everything, and then have a separate pass that removes anything that's None (or maybe "any of these keys, if the value is None"), and then just special-case the requirement for max_results to be positive. But I'd be looking to make a broader change somewhere, taking into account a lot more code, rather than doing this whole "take local names and plonk them into a dict" thing at all. ChrisA

Andrew Barnert

8:58 p.m.

On Sep 8, 2019, at 11:34, Chris Angelico wrote:

...

You're right that the triple repeated name is not caused by your style. However, your suggestion also doesn't solve it. What I'd probably want to do, here, is build the dictionary with everything, and then have a separate pass that removes anything that's None (or maybe "any of these keys, if the value is None"), and then just special-case the requirement for max_results to be positive. But I'd be looking to make a broader change somewhere, taking into account a lot more code, rather than doing this whole "take local names and plonk them into a dict" thing at all.

If you really want to avoid repeating names, the only plausible option seems to be putting them in a namespace that you can then filter and dictify, instead of storing them as a bunch of separate locals. Well, I suppose you could use locals itself as that namespace: query_data = {k: v for (k, v) in locals().items() if k in {'max_results', 'active', 'deleted'} and v} … but that’s pretty horrible. And I suppose there is something Python could do to alleviate the repetitions. The stalled proposal for magic keyword arguments would let you write something like this: query_data = dict(max_results=, active=, deleted=) … which you could then filter. But I don’t think most people liked that proposal, and I don’t think this is a good enough use case to revive it.

Rob Cliffe

9 Sep 9 Sep

10:06 a.m.

...

...
On 07/09/2019 18:59:49, Chris Angelico wrote:

...
On Sat, Sep 7, 2019 at 11:27 PM Rob Cliffe via Python-ideas wrote:

...
A chance for me to bang the drum on one of my pet themes: Sometimes the readability of code is improved by breaking the sacred taboo of 1 statement per line, if it allows similar constructs to be vertically aligned:

if max_results > 0 : query['max_results'] = max_results if active is not None : query['active'] = active if deleted is not None: query['deleted'] = deleted

If this comes out ragged in your browser, the colons and equal signs are meant to be vertically aligned. I'm not bothered by putting an if and a simple statement together, but I am definitely bothered by the need to vertically align them, and especially by the triple repeated name. Not a fan of this style - looks like a bug magnet and maintenance burden.

ChrisA The OP's code:

if max_results > 0: query['max_results'] = max_results if active is not None: query['active'] = active if deleted is not None: query['deleted'] = deleted

is perfectly reasonable and has the same triple repeated name. Would you rewrite it differently, and if so, how? I'm just saying that aligning similar elements and bringing them together, without intervening lines, makes it easier to see the code structure and spot typos. Not to mention using less vertical screen space. I don't understand why you think it is a bug magnet and maintenance burden. I think it is *easier* to maintain: in which of the following is it easier to spot the mistake?

# Version 1: if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

# Version 2: if max_results > 0: querydata['max_results'] = max_results if active is not None: querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

Data point: I have used this style repeatedly in my code (only where appropriate, of course) and find it helpful. Now add a fourth one, for result_count. What does your diff look like? Do you have to edit every other line just to realign it? That's the

On Mon, Sep 9, 2019 at 4:13 AM Rob Cliffe via Python-ideas wrote: maintenance burden - fiddling around with formatting.

Honestly, I couldn't see the mistake in _either_ version until I looked pretty closely, upon which it was equally visible in both. If I were using this in a full app, it would be something a linter could easily spot, again regardless of the alignment. I should have used a more obvious typo than changing a "y" into a "v",

On 08/09/2019 19:34:33, Chris Angelico wrote: they look too similar: if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: querodata['deleted'] = deleted Can you honestly say it's not easier to spot the inconsistency when elements that should be identical are adjacent? (BTW I changed the identifier from query to querydata because my spell-checker, and maybe yours, highlights the mistake instantly in both versions.) Rob Cliffe

...

You're right that the triple repeated name is not caused by your style. However, your suggestion also doesn't solve it. What I'd probably want to do, here, is build the dictionary with everything, and then have a separate pass that removes anything that's None (or maybe "any of these keys, if the value is None"), and then just special-case the requirement for max_results to be positive. But I'd be looking to make a broader change somewhere, taking into account a lot more code, rather than doing this whole "take local names and plonk them into a dict" thing at all.

ChrisA _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-leave@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/65SSUR... Code of Conduct: http://python.org/psf/codeofconduct/

--- This email has been checked for viruses by AVG. https://www.avg.com

Steven D'Aprano

10 Sep 10 Sep

12:43 a.m.

On Sat, Sep 07, 2019 at 08:22:58PM +0100, Rob Cliffe via Python-ideas wrote:

...

in which of the following is it easier to spot the mistake?

# Version 1: if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

# Version 2: if max_results > 0: querydata['max_results'] = max_results if active is not None: querydata['active'] = active if deleted is not None: quervdata['deleted'] = deleted

Obviously the mistake is that max_results is supposed to be greater than OR EQUAL to zero. *wink* But seriously, most coding errors aren't simple spelling errors. If they are, chances are that you will immediately get a NameError, or (as you point out yourself) your spell checker will spot it. The insidious errors are logic errors and this does not help with those unless your lines are almost identical: if seq[0] == 1: query[0] == 0 if seq[1] == 1: query[1] == 1 if seq[2] == 1: query[2] == 2 in which case you should probably use a for loop. If this style works for you, I'm glad for you and don't wish to persuade you differently. But for me, I am not comfortable with this style and would not use it because I have seen far too many people (including myself) waste too much time farting about fixing formatting and alignment issues instead of doing productive work. # Oh no now I have to re-align ALL THE THINGS if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None: querydata['deleted'] = deleted if items and items[0] >= items[-1]: querydata['items'] = items if max_results > 0 : querydata['max_results'] = max_results if active is not None : querydata['active'] = active if deleted is not None : querydata['deleted'] = deleted if items and items[0] >= items[-1]: querydata['items'] = items Not only is a time-waster, it messes up your diffs, forces short lines to be as long as long lines (increasing the chances you will exceed 80 columns), and fills your code with rivers of whitespace in the middle of the text. -- Steven

Andrew Barnert

1:51 a.m.

On Sep 9, 2019, at 17:43, Steven D'Aprano wrote:

...

But for me, I am not comfortable with this style and would not use it because I have seen far too many people (including myself) waste too much time farting about fixing formatting and alignment issues instead of doing productive work.

To be fair, you can use editor plugins to manage this stuff. For example, I used to have a mess of emacs lisp that I maintained that turned exactly your “oh no I have to reformat all the things” case into “ctrl-c :”. But I no longer use any of that. I’m much happier since I set up the black auto-formatter as a save hook. I can no longer do fancy things like lining up the colons, so I no longer waste time doing fancy things like lining up the colons (or writing lisp code to do it for me). Occasionally, it does turn a complicated expression into an unreadable mess, but then I just break the statement up into two statements that it can handle just fine, and the result is almost always easier to follow anyway. The only problem is that if I’m editing someone else’s code who doesn’t use black, I have to disable my save hook so I don’t accidentally commit a diff that destroys all their fancy lined up colons. Anyway, back on topic: earlier I facetiously suggested that you could simplify this code by replacing the mess of local variables with a simple namespace that holds them all, then dictifying and filtering that… but actually, I’m not sure that’s a joke anymore. Why _should_ all those things be in a whole mess of locals, if the main thing you’re using them for is to create a config dict or JSON response or whatever? They probably are conceptually attributes of some object, and if the object is too dumb to be worth declaring a custom class for it and writing a for_json or as_dict or whatever, you can just use a SimpleNamespace.

1689

Age (days ago)

1695

Last active (days ago)

List overview

Download

23 comments

13 participants

participants (13)

Andrew Barnert
Brandt Bucher
brian.skinn＠gmail.com
Chris Angelico
Guido van Rossum
Joao S. O. Bueno
Paul Moore
Rich Smith
Richard Musil
rls＠hwyl.org
Rob Cliffe
Sebastian Kreft
Steven D'Aprano

Conditional dict declarations

tags

participants (13)