fixed toml parser ignoring style

[jspaezp]

Closes #155

[codecov]

Codecov Report

Merging #156 (f4ee706) into main (c749fc6) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #156   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines          582       589    +7     
=========================================
+ Hits           582       589    +7     

Impacted Files	Coverage Δ
...ydocstringformatter/_configuration/toml_parsing.py	100.00% <100.00%> (ø)

[Pierre-Sassoulas]

Thank you for opening a pull request. Could you add covering tests, please ?

[jspaezp]

@Pierre-Sassoulas Sure thing! Done! Added testing that checks if the style is applied correctly depending on the toml file.

Regarding the the added lines marked as not being covered (not implemented error) by definition cannot be covered.
It will be raised if the implementation of the arguments changes in a way that is not supported by the parser (without it, it will fail silently instead of raising an error)

[DanielNoord]

Regarding the the added lines marked as not being covered (not implemented error) by definition cannot be covered. It will be raised if the implementation of the arguments changes in a way that is not supported by the parser (without it, it will fail silently instead of raising an error)

You can add a # pragma: no cover comment to this line to force the coverage to pass there.

Could you also take a look at the failing pre-commit run?

[jspaezp]

Regarding the the added lines marked as not being covered (not implemented error) by definition cannot be covered. It will be raised if the implementation of the arguments changes in a way that is not supported by the parser (without it, it will fail silently instead of raising an error)

You can add a # pragma: no cover comment to this line to force the coverage to pass there.

Could you also take a look at the failing pre-commit run?

I am not sure why the mypy pre-commit is failing, it fails with error:

pydocstringformatter/_configuration/toml_parsing.py:52: error: Module has no attribute "_ExtendAction"; maybe "_AppendAction"?  [attr-defined]
Found 1 error in 1 file (checked 37 source files)

which is odd ... since it has been added since python 3.8 (and the fact that the code runs, if the object did not exist it would result in a runtime error):

python/cpython@aa32a7e

[Pierre-Sassoulas]

LGTM, we need to silence the mypy error if it's passing all the test imo.

[DanielNoord]

LGTM, we need to silence the mypy error if it's passing all the test imo.

Hmm, I don't really agree. It seems as if the pre-commit environment uses python3. That could be an older version.

We should at least try setting the pre-commit.ci environment to use 3.10 and see if that helps.

[Pierre-Sassoulas]

We should at least try setting the pre-commit.ci environment to use 3.10 and see if that helps.

I think that's the default right now ? Otherwise the option is language_version: python3.10

[jspaezp]

I have tried several configurations to set the python version and none seems to fix the error.
Even locally installing mypy and passing the arguments ...

$   python -m mypy --config-file=pyproject.toml
pydocstringformatter/_configuration/toml_parsing.py:6: error: Module "argparse" has no attribute "_ExtendAction"; maybe "_AppendAction"?  [attr-defined]
pydocstringformatter/_testutils/primer/packages.py:57: error: Module has no attribute "Repo"  [attr-defined]
Found 2 errors in 2 files (checked 34 source files)

$   python -m mypy --config-file=pyproject.toml --python-version=3.10
pydocstringformatter/_configuration/toml_parsing.py:6: error: Module "argparse" has no attribute "_ExtendAction"; maybe "_AppendAction"?  [attr-defined]
pydocstringformatter/_testutils/primer/packages.py:57: error: Module has no attribute "Repo"  [attr-defined]
Found 2 errors in 2 files (checked 34 source files)

$   python -m mypy --version                                      
mypy 0.971 (compiled: yes)

$  python --version
Python 3.10.4

$   python -c "import argparse ; print(argparse._ExtendAction)"
<class 'argparse._ExtendAction'>

$   python -c "import git ; print(git.Repo)"
<class 'git.repo.base.Repo'>

[DanielNoord]

Oh wow, it seems like typeshed hasn't been updated for this.

See https://github.com/python/typeshed/blob/master/stdlib/argparse.pyi.

If you want you can open an issue or PR for this against typeshed. I think they would appreciated that.

I'll try and get a review of this PR tonight. Thanks for the testing you already did @jspaezp!

[jspaezp]

Wow! I did not know about typeshed!

Just for reference in the future:
PR adding _ExtendAction to typeshed:
python/typeshed#8701

It got approved! so in time mypy will work correctly

[DanielNoord]

Wow! I did not know about typeshed!

Just for reference in the future: PR adding _ExtendAction to typeshed: python/typeshed#8701

It got approved! so in time mypy will work correctly

Congratulations on your first contribution! 🎉

[Pierre-Sassoulas]

It got approved! so in time mypy will work correctly

This is also going to impact pylint 😄

python/typeshed#8701 (comment)

[DanielNoord]

Thank you very much @jspaezp for all the work here.

Sadly this PR unearthed another issue within the tool that (after being addressed) will break the current test. I have given some pointers to making a new test. If you need any help please let me know!

[github-actions]

According to the primer, this change has no effect on the checked open source code. 🤖🎉

[DanielNoord]

Thanks @jspaezp 😄