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

Format bytes string #6166

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Format bytes string #6166

merged 1 commit into from
Jul 31, 2023

Conversation

lkh42t
Copy link
Contributor

@lkh42t lkh42t commented Jul 29, 2023

Summary

Format bytes string

Closes #6064

Test Plan

Added a fixture based on string's one

@github-actions
Copy link
Contributor

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.02      8.6±0.19ms     4.7 MB/sec    1.00      8.5±0.12ms     4.8 MB/sec
formatter/numpy/ctypeslib.py               1.00  1656.8±21.32µs    10.1 MB/sec    1.01  1674.6±29.60µs     9.9 MB/sec
formatter/numpy/globals.py                 1.00    179.6±1.77µs    16.4 MB/sec    1.03    184.6±0.34µs    16.0 MB/sec
formatter/pydantic/types.py                1.00      3.6±0.03ms     7.2 MB/sec    1.01      3.6±0.08ms     7.1 MB/sec
linter/all-rules/large/dataset.py          1.00     11.1±0.03ms     3.7 MB/sec    1.00     11.2±0.05ms     3.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.8±0.01ms     5.9 MB/sec    1.00      2.8±0.01ms     5.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    385.4±0.65µs     7.7 MB/sec    1.00    385.2±0.55µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.0±0.02ms     5.1 MB/sec    1.00      5.0±0.11ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.00      5.9±0.03ms     6.9 MB/sec    1.01      6.0±0.03ms     6.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1247.3±8.86µs    13.4 MB/sec    1.00   1250.3±5.63µs    13.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    137.3±1.22µs    21.5 MB/sec    1.01    138.3±0.70µs    21.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.6±0.01ms     9.7 MB/sec    1.01      2.6±0.03ms     9.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01     10.2±0.16ms     4.0 MB/sec    1.00     10.2±0.10ms     4.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1969.6±26.67µs     8.5 MB/sec    1.00  1972.5±22.45µs     8.4 MB/sec
formatter/numpy/globals.py                 1.00    216.8±6.12µs    13.6 MB/sec    1.02    220.0±8.65µs    13.4 MB/sec
formatter/pydantic/types.py                1.01      4.3±0.13ms     5.9 MB/sec    1.00      4.3±0.05ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     13.6±0.11ms     3.0 MB/sec    1.00     13.5±0.12ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.04ms     4.7 MB/sec    1.00      3.6±0.05ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    431.6±5.80µs     6.8 MB/sec    1.00    433.6±7.08µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.1±0.10ms     4.1 MB/sec    1.00      6.1±0.06ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.5±0.06ms     5.4 MB/sec    1.00      7.5±0.06ms     5.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1534.5±30.48µs    10.9 MB/sec    1.00  1513.9±12.86µs    11.0 MB/sec
linter/default-rules/numpy/globals.py      1.01    169.6±3.49µs    17.4 MB/sec    1.00    167.5±2.75µs    17.6 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.3±0.05ms     7.7 MB/sec    1.00      3.3±0.04ms     7.8 MB/sec

@konstin konstin mentioned this pull request Jul 31, 2023
72 tasks
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you

@@ -51,16 +51,13 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
Constant::Int(_) => FormatInt::new(item).fmt(f),
Constant::Float(_) => FormatFloat::new(item).fmt(f),
Constant::Complex { .. } => FormatComplex::new(item).fmt(f),
Constant::Str(_) => {
Constant::Str(_) | Constant::Bytes(_) => {
let string_layout = match self.layout {
ExprConstantLayout::Default => StringLayout::Default,
ExprConstantLayout::String(layout) => layout,
};
FormatString::new(item).with_layout(string_layout).fmt(f)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin I'm a bit surprised that black also normalizes byte strings because I assumed we would need to treat them as opaque bytes. Are you aware of any normalization that isn't safe for byte strings?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as i know and reading https://docs.python.org/3/library/stdtypes.html#bytes, byte strings are pretty much like normal strings except that they are limited to ascii input, allow arbitrary bytes and \x behaves differently

@MichaReiser MichaReiser requested a review from konstin July 31, 2023 08:15
@konstin
Copy link
Member

konstin commented Jul 31, 2023

Found a bug in raw string formatting bug while reviewing, but it's a preexisting general string formatting bug and not your fault (#6186). Fixing #6186 however should add tests both for regular strings and for byte strings.

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@konstin konstin merged commit b95fc6d into astral-sh:main Jul 31, 2023
17 checks passed
@lkh42t lkh42t deleted the format-bytes branch July 31, 2023 08:48
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.

Bytes string formatting
3 participants