Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update p4_branch #3

Merged
merged 54 commits into from
Apr 26, 2021
Merged

Update p4_branch #3

merged 54 commits into from
Apr 26, 2021

Conversation

thaprau
Copy link
Owner

@thaprau thaprau commented Apr 26, 2021

No description provided.

lachnerm and others added 30 commits April 10, 2021 13:15
It appears that the linking from the column titles of the table of the 
Summary tab to the Quantile plot has been broken for a while now. Left 
over code fragments from this were removed and the links were fixed.
…t-links

Fix links to quantile plot from summary table
That class creates the directory in its constructor,
so let's also remove it there in the close() method.
If we did not start benchmarking at all
(for example because tool was not found),
the call to benchmark.tool.close() was missing.
This meant that the multiprocessing pool for the tool-info module
was not closed and produced an ugly (but harmless) stack trace
on process termination.
We can make use of subprocess.run() and universal_newlines=True
in most cases to avoid for example manual decode() calls.
We now use subprocess.run and universal_newlines=True.
This also removes the last call to decode_to_string,
but we keep it if some external tool-info modules still call it.
This seems possible now due to new react-scripts version.
We actually do not use it anymore since 4f562f3
The former is included in Python's standard library.
This adds trailing commas in function signatures,
which is good for reducing diff noise.
The "obvious" way to write the code was not supported on Python 3.5,
so we had to use a fallback.
This syntax was introduced in Python 3.6.
PhilippWendler and others added 24 commits April 22, 2021 08:30
glob.glob actually does not provide a way to match
for example either "2" or "10",
the pattern we created in such a case would match either "2", "1", "0", or ",".
This could lead to matching the wrong cores
when cores with multi-digit numbers were used.
The path to table-generator was not escaped at all,
so even whitespace was a problem,
and the paths to the XML files were wrong in case they contained
a ' character.
Now we use a proper escaping method,
which also avoids quotes in the common case where they are not needed.
(e.g., that all memory regions have the same amount of cores)
This was added in Python 3.6 and is nicer to read.
This syntax from Python 3.6 is much nicer to read.

In this commit, only single-line expressions are converted.
The changes were automatically created with flynt.
Created automatically with flynt
and then manually edited to keep to line length.
Introducing an auxiliary variable often makes the code easier to read
and better understandable.
Cases like "\n".join(...) are not allowed in f-strings due to the
backslash. However, for appending a suffix string concatenation is also
fine and the resulting code might actually even be easier to read
than before.
These were cases that flynt did not touch because the expressions were
too complex, so I did it manually.
This is directly supported by the logging module
and evaluated lazily.
Fixing line length, indentation, and unnecessary string concatenation.
@thaprau thaprau merged commit e58fb89 into bench_p4 Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants