parse_args() used in hook.py

[mattbwilmes]

In hook.py, there is a call to parse_args(), which assumes that flake8 is being called from the command line: https://github.com/john-hen/Flake8-pyproject/blob/d4064f6703b113590e175817f2fe2cbb466ffdc1/flake8p/hook.py#L38

However, if someone calls flake8.main.cli.main() using a separate script that is called with command line options other than those used by flake8, this will trigger an error.

Preferred Approach

It would be better to accept the argv passed into flake8.main.cli.main() and call arguments = parse_known_args(argv) before checking for arguments.toml_config

Backup Approach

A less ideal solution that would be a quicker temporary fix would be to use parse_known_args() to call arguments, _ = parse_known_args() before checking for arguments.toml_config

[john-hen]

Yeah, okay. Though I think it's a reasonable assumption that Flake8 would be called from the command line. But we shouldn't throw errors if for some reason it isn't.

I like the backup approach better, because it's less invasive. Otherwise we'd have to monkey-patch some other part of Flake8 as well to get access to its argv. Right? It's not available in the parse_config function that we're patching right now. So this would add more complexity. (Then again, anything that passes the test suite and doesn't like double the lines of code is probably fine.)

Do you want to create a PR?

[mattbwilmes]

I agree it's more common to call flake8 from the command line, but if it needs to be called from within a script (as is my case), it is a better practice to import and call flake8 instead of using subprocess.run() (see https://stackoverflow.com/questions/48862112/subprocess-or-import-to-invoke-a-script-in-python).

My solution passes the test suite and added a net of 9 lines of code and 10 lines of documentation/comments. However, as you predicted, it involved overriding yet another of flake8's functions, so let me know what you think.

[john-hen]

Thanks for the PR. I'll merge and release soon, but see my notes there.

Since you brought it up... That Stack Overflow question is not really relevant here. Given that Flake8 does not have a public Python API, you call something like flake8.main.cli.main() at your own risk. But you have no guarantee that that function won't ever disappear between versions. It's unlikely, sure. But with command-line tools, you'd typically rely on the actual command-line interface, i.e. start it as a subprocess. Pip would be another example. We're not supposed to import pip.

Then again, obvioulsy this package here does "much worse", as we're monkey-patching Flake8's internals.

[mattbwilmes]

Thanks for the PR. I'll merge and release soon, but see my notes there.

Issue fixed, thanks for pointing that out :smiley:

Since you brought it up... That Stack Overflow question is not really relevant here. Given that Flake8 does not have a public Python API, you call something like flake8.main.cli.main() at your own risk. But you have no guarantee that that function won't ever disappear between versions. It's unlikely, sure. But with command-line tools, you'd typically rely on the actual command-line interface, i.e. start it as a subprocess. Pip would be another example. We're not supposed to import pip.

Then again, obvioulsy this package here does "much worse", as we're monkey-patching Flake8's internals.

I appreciate the warning, that's a fair point regarding the lack of a public API and something I will keep in mind for my script for future versions of flake8 :+1: . I guess this is something to keep in mind for this plug-in as well since it is setting main = flake8.main.cli.main here.

[john-hen]

Like I said, we're using dirty tricks, because there is no other way. But note how the test suite doesn't: It calls Flake8 as a subprocess.

[john-hen]

Fixed in version 1.2.3, released today.