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 terminal.py #231

Merged
merged 2 commits into from
Dec 3, 2021
Merged

Update terminal.py #231

merged 2 commits into from
Dec 3, 2021

Conversation

mwchase
Copy link

@mwchase mwchase commented Nov 30, 2021

Attempt to make pylint happy with the kwargs documentation.

This line broke my other PR, so if this passes, it should be good.

Attempt to make pylint happy with the kwargs documentation
@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #231 (1ae14c4) into master (08417d6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #231   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files           9        9           
  Lines        1014     1014           
  Branches      176      176           
=======================================
  Hits          967      967           
  Misses         43       43           
  Partials        4        4           
Impacted Files Coverage Δ
blessed/terminal.py 98.25% <ø> (ø)

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 08417d6...1ae14c4. Read the comment docs.

@avylove
Copy link
Collaborator

avylove commented Nov 30, 2021

I think pydocstyle wants to start the docstring with r""" if it has backslashes.
Looking at the documentation produced, Sphinx wants a single backslash.
I think there is a bug in Pylint that isn't removing the escapes before comparing the strings. Worth reaching out to them with an issue to see if it's a quick fix. If not, we may just need to ignore this error.

Fix pycodestyle error.

We'll see if this helps pylint as well.
@mwchase
Copy link
Author

mwchase commented Nov 30, 2021

Sorry about the obvious failure. I was being a little lazy, and just doing these updates through the web interface. (They seemed so simple, but I guess not.)

@mwchase
Copy link
Author

mwchase commented Nov 30, 2021

Just confirmed locally that Sphinx does not like having the backslash removed. I investigated on the pylint side, and it looks like this is pylint-dev/pylint#5406.

Thinking about what to do in terms of getting things to pass, my inclination would be to revert the changes to the docstring, and either version-lock pylint below 2.12, or add inline disables to the function definition, though I'm happy to do whatever. Any preferences?

@avylove
Copy link
Collaborator

avylove commented Dec 1, 2021

I don't think we're in a hurry. Let's see what the Pylint guys say. They tend to fix things like this pretty quickly. If it becomes a blocker we'll just inline disable that check, but we can hold off for now.

@mwchase
Copy link
Author

mwchase commented Dec 3, 2021

I just confirmed that the new pylint release passes accepts these changes, so this should be good to try again whenever.

Copy link
Collaborator

@avylove avylove left a comment

Choose a reason for hiding this comment

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

Looks good to me

@avylove avylove merged commit 9c81ce7 into jquast:master Dec 3, 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.

None yet

2 participants