Minor optimizations in Python code

[cclauss]

% ruff check --select=C4,PERF,PIE810 --statistics | grep "\[\*\]"

Edit as requested below:

Ruff has two main command verbs: check to lint Python code and format to format code in a way that is highly compatible with psf/black. The --select option allows us to choose which of the 800+ linting rules we wish to apply to the code. Here we focus on minor optimizations. The --statistics option provides only summary information.

The grep "\[\*\]" command says that we are only interested in rules that have automated fixers.

The detailed description of each rule including Why is this bad? can be obtained with a command of the form ruff rule RUF017 as demonstrated below.

24	PIE810 	[*] multiple-starts-ends-with
13	C408   	[*] unnecessary-collection-call
 8	C419   	[*] unnecessary-comprehension-in-call
 5	C416   	[*] unnecessary-comprehension
 2	C400   	[*] unnecessary-generator-list
 2	C413   	[*] unnecessary-call-around-sorted
 2	PERF102	[*] incorrect-dict-iterator
 1	C401   	[*] unnecessary-generator-set
 1	C405   	[*] unnecessary-literal-set
 1	C417   	[*] unnecessary-map

% ruff check --select=C4,PERF,PIE810 --fix --unsafe-fixes

Found 140 errors (58 fixed, 82 remaining).

% ruff rule RUF017

quadratic-list-summation (RUF017)

Derived from the Ruff-specific rules linter.

Fix is always available.

What it does

Checks for the use of sum() to flatten lists of lists, which has
quadratic complexity.

Why is this bad?

The use of sum() to flatten lists of lists is quadratic in the number of
lists, as sum() creates a new list for each element in the summation.

Instead, consider using another method of flattening lists to avoid
quadratic complexity. The following methods are all linear in the number of
lists:

• functools.reduce(operator.iadd, lists, [])
• list(itertools.chain.from_iterable(lists))
• [item for sublist in lists for item in sublist]

When fixing relevant violations, Ruff defaults to the functools.reduce
form, which outperforms the other methods in microbenchmarks.

Example

lists = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
joined = sum(lists, [])

Use instead:

import functools
import operator


lists = [[1, 2, 3], [4, 5, 6], [7, 8, 9]]
functools.reduce(operator.iadd, lists, [])

References

• How Not to Flatten a List of Lists in Python
• How do I make a flat list out of a list of lists?

[eli-schwartz]

I think the commit message could probably be a bit better (nothing huge, but I'd at least include in the extended description the command you used to reproduce the changes).

The other big thing to me is my comment below about the second commit.

[eli-schwartz]

This whole logic feels kinda hairy honestly. I wonder if there's a better way to do it that doesn't need additional imports...

[cclauss]

Hairy indeed. The commit message lists the three linear solutions and why ruff defaults to this one. Given its hairiness, let's revert this change and deal with it in a separate PR.

[eli-schwartz]

Again, please no "Apply suggestions from code review" commits or other forms of fixup for a previous commit in the same PR.

[cclauss]

Can someone please stop and rerun the runaway GitHub Action? Five hours is a bit too long!

[eli-schwartz]

$ git --no-pager range-diff origin/master 3f998e47b 6709c1800
1:  3f998e47b ! 1:  6709c1800 minor optimizations
    @@ Metadata
      ## Commit message ##
         minor optimizations
     
    - ## ci/ciimage/build.py ##
    -@@ ci/ciimage/build.py: class ImageDef:
    -         data = json.loads(path.read_text(encoding='utf-8'))
    - 
    -         assert isinstance(data, dict)
    --        assert all([x in data for x in ['base_image', 'env']])
    -+        assert all(x in data for x in ['base_image', 'env'])
    -         assert isinstance(data['base_image'], str)
    -         assert isinstance(data['env'],  dict)
    - 
    -
      ## docs/jsonvalidator.py ##
     @@ docs/jsonvalidator.py: root: dict

Any particular reason for this? Just curious...

[dcbaker]

The changes here look good to me. Once Eli is happy with everything I'm fine to merge this.

[cclauss]

I reverted ci/ciimage/build.py because the three build_images jobs were failing…. I could not see how this modification would cause tests to fail but out of an abundance of caution…

[eli-schwartz]

They fail in git master. Those jobs are only run on a weekly schedule or whenever the job definition itself changes -- so avoiding to change the job definition, doesn't make any difference to the test other than preventing its status from showing up.

It is perfectly fine to include that change...