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

Upgrade pyupgrade for crlf fixes (again) #4281

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Upgrade pyupgrade for crlf fixes (again) #4281

merged 1 commit into from
Oct 31, 2018

Conversation

asottile
Copy link
Member

I promise I commit things to pytest other than changing this yaml file 😭

@nicoddemus
Copy link
Member

Hahaha thanks for the lightning quick fix @asottile! 🙇

@RonnyPfannschmidt
Copy link
Member

personally i appreciate having someone that's not me dedicated to the pain of yaml ^^

@asottile
Copy link
Member Author

@RonnyPfannschmidt I feel ya, yaml is my biggest regret w/ pre-commit

@nicoddemus
Copy link
Member

yaml is my biggest regret w/ pre-commit

Oh really? I'm curious why you feel that way. Also, which alternatives would you use instead of yaml for pre-commit configuration?

@asottile
Copy link
Member Author

yaml is my biggest regret w/ pre-commit

Oh really? I'm curious why you feel that way. Also, which alternatives would you use instead of yaml for pre-commit configuration?

The biggest being that it can't be programmatically rewritten. autoupdate (though glorious) is a giant hack -- it used to be one of the biggest sources of frustration.

There's also the sheer complexity (and slowness) of yaml -- the spec for json fits on a postcard, but yaml's is 84 pages. The default implementation of yaml has some famous security problems and unless you've got libyaml-dev at install time (and the appropriate code to opt into the fast path, you're stuck in the pure-python slow path. Even the fast path is orders of magnitude slower than the equivalent json calls.

To be honest, I haven't found a better alternative. What I really want is some high level configuration that's fast, readable, and machine rewritable. json5 seems so close to what I want, but the parsers for python are an awful pile of pure python + regex and aren't round trippable. I even tried to invent my own config language and while it worked, it wasn't ever really production worthy (a fun exercise though, I learned a ton and wrote some cool toys).

Maybe one day I'll have enough time to make json5 functional in python... but until then I'll continue shouting to the wind 😭

@codecov
Copy link

codecov bot commented Oct 31, 2018

Codecov Report

Merging #4281 into master will decrease coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4281      +/-   ##
==========================================
- Coverage   95.87%   95.65%   -0.23%     
==========================================
  Files         109      109              
  Lines       24630    24630              
  Branches     2396     2396              
==========================================
- Hits        23615    23560      -55     
- Misses        720      759      +39     
- Partials      295      311      +16
Flag Coverage Δ
#docs ?
#doctesting ?
#linting ?
#linux 95.65% <ø> (+0.01%) ⬆️
#nobyte 91.39% <ø> (-0.64%) ⬇️
#numpy 41.61% <ø> (-51.41%) ⬇️
#pexpect 41.61% <ø> (ø) ⬆️
#py27 93.93% <ø> (-0.13%) ⬇️
#py34 92.22% <ø> (+0.53%) ⬆️
#py35 92.23% <ø> (-0.1%) ⬇️
#py36 93.82% <ø> (-0.18%) ⬇️
#py37 92.37% <ø> (+0.03%) ⬆️
#trial 41.61% <ø> (-51.41%) ⬇️
#windows ?
#xdist 93.74% <ø> (-0.17%) ⬇️
Impacted Files Coverage Δ
testing/test_pathlib.py 91.17% <0%> (-8.83%) ⬇️
src/_pytest/assertion/util.py 92.99% <0%> (-5.15%) ⬇️
testing/test_tmpdir.py 94.33% <0%> (-4.41%) ⬇️
src/_pytest/capture.py 90.2% <0%> (-3.65%) ⬇️
src/_pytest/pathlib.py 87.43% <0%> (-2.19%) ⬇️
testing/acceptance_test.py 97.19% <0%> (-1.08%) ⬇️
src/_pytest/nodes.py 93.95% <0%> (-0.81%) ⬇️
src/_pytest/pytester.py 86.96% <0%> (-0.43%) ⬇️
testing/test_capture.py 98.94% <0%> (-0.31%) ⬇️
src/_pytest/fixtures.py 97.12% <0%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fea71a...af00367. Read the comment docs.

@nicoddemus
Copy link
Member

Thanks for the detailed write up!

And what about toml?

@nicoddemus nicoddemus merged commit 642521a into pytest-dev:master Oct 31, 2018
@asottile asottile deleted the bump_hooks branch November 1, 2018 00:05
@asottile
Copy link
Member Author

asottile commented Nov 1, 2018

Also doesn't have a round-trip parser :(

@nicoddemus
Copy link
Member

Hmm.

About the roundtrip parser, conda uses ruamel.yaml. 👍

@asottile
Copy link
Member Author

asottile commented Nov 1, 2018

Yeah, I tried it it didn't quite cut it. Kinda funny though, from their overview page:

flow style sequences ( ‘a: b, c, d’) (based on request and test by Anthony Sottile)

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.

None yet

3 participants