4 ms·
> But the arguments given in support of the feature were generally fairly weak; they often used arbitrary, "made up" examples that demonstrated a place where mu
by dataflow 4y ago
> But the arguments given in support of the feature were generally fairly weak; they often used arbitrary, "made up" examples that demonstrated a place where multi-level break could be used, but were not particularly compelling.
> To make this proposal convincing, we need a realistic example of an algorithm that uses it, and that example needs to be significantly more readable and maintainable than the refactorings into functions, or the use of try…except (also a localised goto).
How about this:
found = None
for r, row in enumerate(table.rows):
for c, cell in enumerate(row):
if search_query.matches(cell.value):
found = (r, c)
break 2
logger.log("Found cell: {}".format(found))
return found
- gurchik 4y agoThat doesn't look to be "significantly more readable" than: def find_cell(table): for r, row in enumerate(table.rows): for c, cell in enumerate(row): if search_query.matches(cell.value): return (r, c) found = find_cell(table) logger.log("Found cell: {}".format(found))
- jdougan 4y agoI suspect the Pythonistas would suggest something like the below is somehow better. I'm not confident in it's superiority, particularly since python doesn't really optimize. def searchRow(row,r): found = None for c, cell in enumerate(row): if cell.value == expected: found = (r, c) break return found found = None for r, row in enumerate(self.rows): found = searchRow(row, r) if found is not None: break logger.log("Found item: {}".format(found)) return found
- ajanuary 4y agodef _find_cell(table, search_query): for r, row in enumerate(table.rows): for c, cell in enumerate(row): if search_query.matches(cell.value): return (r, c) return None found = _find_cell(table, search_query) logger.log("Found cell: {}".format(found)) return found
- dataflow 4y agoOkay, so we managed to add cruft to the code without making it significantly worse. Now is this a change you would actually prefer as an improvement if someone asked you to review the original version? It may be worth observing Gouvernathor does not exactly write "factored-out" code like this either: https://github.com/Gouvernathor/renpy-ParliamentDiagram/blob/d13387a111b72e7e9a91b96bb912bcb1d7cf7aac/newarch.rpy#L64-L110 https://github.com/Gouvernathor/renpy-ParliamentDiagram/blob...
- ajanuary 4y agoBut not making it significantly worse is the bar requested: > that example needs to be significantly more readable and maintainable Yes, things can be slightly tidier with labelled breaks in some contexts. But that’s not a particularly high bar. There are tons of language features for which that’s true. Adding everyone’s pet nicety to the language makes it a huge language, which isn’t what they’re aiming for. If that’s what you want, go use Perl or C#, both of which are very fine languages.
- dataflow 4y ago> But not making it significantly worse is the bar requested. [...] Adding everyone’s pet nicety to the language makes it a huge language, which isn’t what they’re aiming for. I might risk going on a rant here, but if the improvement from break-label is insignificant enough that it wouldn't meet that bar, I'm not sure how a ton of other features already in the language ever met it. Did for-else and while-else clauses (which somehow pretty much every other language on earth gets by without) genuinely meet this bar? How about the (pitfall-ridden!) match-case and walrus operator that they added quite recently? Did assignments-in-conditionals suddenly meet this bar in 2019 but not meet it in the decades prior where they refused to consider it compelling enough? What about annotations—couldn't you already do f = annotate(f)?
- progval 4y agoYou can use a generator expression: all_matches = ( (r, c) for r, row in enumerate(table.rows) for c, cell in enumerate(row) if search_query.matches(cell.value) ) found = next(all_matches, None) if found: logger.log("Found cell: {}".format(found)) return found It is lazy, so it stops as soon as the first match is found.
- dataflow 4y agoThis doesn't generalize well. Add a statement or something in the loop and it clashes with this.
- joshuamorton 4y agoThere's a number of options here that I'd suggest: first would be that you're doing something wrong with the api of the table, it should be indexable as `table[r][c]` or `table[r, c]`, and you really should opt for that if at all possible, then this becomes super straightforward. But if you really need this to work on an 2-d array that is only an Iterable, and somehow isn't Sized or Sequence, this still works: def flatten(table): for r, row in enumerate(table.rows): for c, cell in enumerate(row): yield (r, c, row, cell) def query(table): for (r, c, row, cell) in flatten(table): if search_query.matches(cell.value): return (r, c) This process is generalizable, you can pretty much always separate the iterate and test steps by following this pattern.
- dataflow 4y ago(1) I dare say this is less readable than the original. Not un-readable, but definitely less readable. Just put them side-by-side if this isn't obvious. (2) You probably don't want to call it something generic like "flatten" if it's going to do something so specific like concatenate table.rows. Note that I'm not just nitpicking on the name here. Rather, my bigger point is your factored-out functions aren't really as reusable as your name suggests. In this particular case you can trivially get around it by passing table.rows directly, but that obviously wouldn't generalize if you needed anything else in the table in the process. > This process is generalizable (3) I'd question that too. Imagine if you had a per-row operation too. Like maybe: found = None for r, row in enumerate(table.rows): for c, cell in enumerate(row): if search_query.matches(cell.value): found = (r, c) break 2 logger.log("Finished row #{}".format(r + 1)) Are you really going to factor it out like this? def flatten(table): for r, row in enumerate(table.rows): for c, cell in enumerate(row): yield (r, c, row, cell) logger.log("Finished row #{}".format(r + 1))
- joshuamorton 4y ago(1) yes my way is clearer, side by side this is very obvious. Certainly it is not worse. (2) sure, but this doesn't change anything. Factoring a big ugly function out into two smaller easy functions is...fine, even if each is only used once. The name I picked wasn't to imply generalization, it was because I needed a name. (3) yes. But again the whole reason this is unreadable at all is because you've created an intentionally obtuse api. We're returning the row and column, so clearly either were able to index into this thing normally, or it's a temp we've created, in which case we can modify it's api and index into it normally.
- BiteCode_dev 4y agoEDIT: made a mistake, see my next comment bellow. I won't update this comment so that other readers can see the code for the 2 use cases. Python has itertools.product and unpacking for this case: from itertools import product space = product(enumerate(table.rows), enumerate(row)) for (r, row), (c, cell) in space: if search_query.matches(cell.value): logger.info("Found cell: (%s,%s)", r, c) return (r, c) Note that we don't need the "found" variable neither to get the same beheavior. We can also use break + else, but in this case, the return is cleaner since that's the default value for python functions. Additionnally, the logger should do the interpolation, not format(). I think the Python team is having to do the difficult job of filtering new feature, and it makes sense they don't introduce it in this case. We already have all the facilities in the language to deal with most nested loop cases, and those are rare cases anyway.
- progval 4y agoNot in this case. 'row' is an item in the 'table.rows' iterable.
- BiteCode_dev 4y agoRight, I misread the code, my bad. For this case, one should use a nested comprehension: space = ((r, c, cell) for r, row in enumerate(table.rows) for c, cell in enumerate(row)) for (r, c, cell) in space: if search_query.matches(cell.value): logger.info("Found cell: (%s,%s)", r, c) return (r, c) Or see masklinn comment if you don't need the logger. In all languages, you separate code for preparing your input and code for processing it. In python, it's also a good idea to do that at the loop level: - prepare your stream of data so that it's normalized using itertools, generators, a function, a numpy array, etc - do a single loop pass for your processing This way you will you obtain a single iterable, and will be able to get the most out of the language tooling: - you can pass it to map/filter, itertools or another comprehension - you can convert to a list, a tuble, a Counter, etc. And slice it - you can use "else" in the loop, which is an underused construct, but very useful when you need break - you can use advanced unpacking - you can call iter() and/or next(). This is what masklinn does in his example. The point being, nested loops are rare enough, and we have tool to deal with them in an idiomatic way in Python, and not adding new syntax for a rare case that usually has a good solution makes sense to me.
- dotancohen 4y agoThrow it in a function and return early: found = findIt(search_query, table.rows, logger) def findIt(query, rows, logger): for r, row in enumerate(rows): for c, cell in enumerate(row): if query.matches(cell.value): logger.log("Found cell: {}".format(found)) return (r, c) break 2 return None
- jasonhansel 4y agoWouldn't you just write: found = next(( (r, c) for r, row in enumerate(rows) for c, cell in enumerate(row) if search_query.matches(cell.value) ), None)