Skip to content

Conversation

@nicolamarella
Copy link

@nicolamarella nicolamarella commented Jan 22, 2022

Closes #15609

Recently I couldn't see errors and warning from pylama linter.
I believe there was a small change in the output format for parsable in pylama, the regex for parse the output is missing a semicolon. See https://github.com/klen/pylama/blob/227ddc94eb73638d423833075b1af2ac6bb308f7/pylama/main.py#L17https://github.com/klen/pylama/blob/227ddc94eb73638d423833075b1af2ac6bb308f7/pylama/main.py#L17
This should fix it.

MyPy output

I added, additionally, the case of mypy output, not having that the error code. See #15609 (comment)

@nicolamarella nicolamarella changed the title Fixed regex for Pylama output WIP: Fixed regex for Pylama output Jan 22, 2022
@nicolamarella nicolamarella changed the title WIP: Fixed regex for Pylama output Fixed regex for Pylama output Jan 22, 2022
@Guy-Markman
Copy link

I hope they will accept PR soon because it had bothered me too

Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Please add a news entry thanking yourself! (see news/README.md for instructions)

@ghost
Copy link

ghost commented Feb 2, 2022

CLA assistant check
All CLA requirements met.

@@ -0,0 +1 @@
Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) No newline at end of file
Copy link

Choose a reason for hiding this comment

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

How does this influence mypy?

Copy link
Author

Choose a reason for hiding this comment

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

If you enable mypy as linters for pylama, it comes without an error code. That is why, in the regex, it is now optional

Copy link
Author

Choose a reason for hiding this comment

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

Copy link

@karrtikr karrtikr Feb 2, 2022

Choose a reason for hiding this comment

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

Sorry allow to me clarify, here's my understanding:

  • MyPy output parsing is handled my the mypy linter.
  • mypy is a separate linter (handled by mypy.ts which has it's own regex) and Pylama is another linter (handled in pylama.ts), so I'm not sure why changes in pylama linter would affect functioning of another linter.

So this shouldn't influence "mypy output parsing", can you clarify?

Copy link
Author

Choose a reason for hiding this comment

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

If you use pylama, you can specify multiple external linters with the option --linters=mccabe,mypy,pycodestyle,pylint,isort.
In this case, the output for those linters is picked up by pylama itself and then re-printed unified under the pylama output.

Choose a reason for hiding this comment

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

Got it, perhaps the following description suits better then:

Suggested change
Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella))
Fixes Pylama output parsing with MyPy (thanks [Nicola Marella](https://github.com/nicolamarella))

@karrtikr karrtikr self-requested a review February 2, 2022 14:04
@karrtikr karrtikr added the skip tests Updates to tests unnecessary label Feb 2, 2022
import { BaseLinter } from './baseLinter';
import { ILintMessage, LintMessageSeverity } from './types';

const REGEX =

Choose a reason for hiding this comment

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

Can you add an example in the comments justifying this regex? Ideally we should be adding tests but seeing as there're currently no tests for this, just the comment would suffice.

@@ -0,0 +1 @@
Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella)) No newline at end of file

Choose a reason for hiding this comment

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

Got it, perhaps the following description suits better then:

Suggested change
Fixes Pylama and MyPy output parsing (thanks [Nicola Marella](https://github.com/nicolamarella))
Fixes Pylama output parsing with MyPy (thanks [Nicola Marella](https://github.com/nicolamarella))

Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

These changes will be good but not required, so I'll be merging this PR, thanks for contributing!

@karrtikr karrtikr merged commit 601777c into microsoft:main Feb 23, 2022
@nicolamarella
Copy link
Author

Hey @karrtikr,
first of all thanks for the review and approval.
I saw your comments but I didn't have time to do the small changes yet, and saw that you merged already. Shall I later open a new PR for those?

@karrtikr
Copy link

Sure, that would be great, just mention me once you are done.

wesm pushed a commit to posit-dev/positron that referenced this pull request Mar 28, 2024
* Fixed regex for pylama output

* added optional error code as it is not reported when using mypy

* added news
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pylama [mypy] linter messages don't appear in the "Problems" tab

3 participants