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

Incorrect reading of pyproject.toml #155

Closed
jspaezp opened this issue Sep 7, 2022 · 1 comment · Fixed by #156
Closed

Incorrect reading of pyproject.toml #155

jspaezp opened this issue Sep 7, 2022 · 1 comment · Fixed by #156
Milestone

Comments

@jspaezp
Copy link
Contributor

jspaezp commented Sep 7, 2022

Hello there!

I found a bug where the 'style' argument is ignored from the configuration file

This is my the section of my toml

[tool.pydocstringformatter]
style = "randomdoc"
exclude = [".tox/**"]
max-line-length = 88

After a little lurking I think it is caused by this ....

The style arg is defined as an extend.

self.configuration_group.add_argument(
"--style",
action="extend",
type=str,
nargs="+",
choices=["pep257", "numpydoc"],
help="Docstring styles that are used in the project. Can be more than one.",
)

And extend style args are not checked in the parsing of the toml file. Thus replacing whatever is in the field with
an empty list (which gets ignored later)

if isinstance(action, argparse._StoreTrueAction):
if value is True:
return [action.option_strings[0]]
return []
if isinstance(action, argparse._StoreAction):
if isinstance(value, int):
value = str(value)
return [action.option_strings[0], value]
return [] # pragma: no cover

Possible fix:

changing the pydocstringformatter/_configuration/toml_parsing.py#L44-L52 to ...

 if isinstance(action, argparse._StoreTrueAction): 
     if value is True: 
         return [action.option_strings[0]] 
     return [] 
 elif isinstance(action, argparse._StoreAction): 
     if isinstance(value, int): 
         value = str(value) 
     return [action.option_strings[0], value] 
 elif isinstance(action, argparse._ExtendAction): 
     if isinstance(value, str): 
         value = str(value) 
     return [action.option_strings[0], value] 
 else:
    raise NotImplementedError
# return []  # pragma: no cover

so it 1 Checks for the extend type args and 2 makes maintaining the package easier because it throws an error instead of silently ignored wrongly parsed files.

btw ... debugging was not super easy to be honest, is there a way to have the cli return the configuration that will be used? (explicit version of all the configuration arguments) ... like --verbose argument.

Let me know if you would like me to make PR for any of these

Best!
Sebastian

@Pierre-Sassoulas
Copy link
Collaborator

Hello, thank you for the debugging and analyzing the issue ! PR are welcome, let's do one for each issue (easier debugging and the configuration bug) .

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 a pull request may close this issue.

2 participants