mkdocstrings / pytkdocs

Load Python objects documentation.
https://mkdocstrings.github.io/pytkdocs
ISC License
50 stars 32 forks source link

Bugfix duplicate dataclass attributes #100

Closed bstadlbauer closed 3 years ago

bstadlbauer commented 3 years ago

This should fix the issue of dataclasses having duplicate attributes when these have defaults. This occurs because attributes with defaults are interpreted differently by Python (as they have a default, they already "exist" in the "regular" class).

The solution implemented here first checks if the argument is already present in self.arguments (comparison by name). If it is, it replaces the old argument (as the new one has the dataclass_field property set).

bstadlbauer commented 3 years ago

@pawamoy Note that poetry environment setup is currently not possible due to a bug: https://github.com/python-poetry/poetry/issues/534

My fix locally was to pin pytest-randomly = "3.5.0"

pawamoy commented 3 years ago

Thanks a lot @bstadlbauer!

Note that poetry environment setup is currently not possible due to a bug: python-poetry/poetry#534

Yes I saw that too yesterday and used the same fix in other projects :slightly_smiling_face:

Your PR is LGTM! A nice fix with few changes :slightly_smiling_face: Would you like to me to fix CI so you can rebase? Or you're OK with merging immediately?

bstadlbauer commented 3 years ago

It would be nice if you could fix CI cause I had six tests failing locally due to a weird import error :-)

Also, i hope it was ok to add marshmallow as a dev dependency?

pawamoy commented 3 years ago

Alright, lets fix that then! And yes, thank you for adding marshmallow, I guess I had it locally installed without Poetry knowing.

pawamoy commented 3 years ago

OK you can rebase on master to get the Poetry/pytest-randomly fix :slightly_smiling_face:

bstadlbauer commented 3 years ago

Rebased, but it looks as if the tests i saw failing locally are also failing here (and I think on master aswell).

Also, it seems as if code quality checking is exiting with a non-zero status code, but the test is marked as ok?

Run poetry run duty check-code-quality
  poetry run duty check-code-quality
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    LANG: en_US.utf-8
    LC_ALL: en_US.utf-8
    POETRY_VIRTUALENVS_IN_PROJECT: true
    PYTHONIOENCODING: UTF-8
    pythonLocation: /opt/hostedtoolcache/Python/3.8.8/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.8.8/x64/lib
> Checking code quality
✗ Checking code quality (1)
bstadlbauer commented 3 years ago

Note: Just checked and seems as if there are 125 linting errors on master at the moment:

➜  pytkdocs git:(master) ✗ poetry run flakehell lint src | wc -l 
     125
pawamoy commented 3 years ago

Don't pay attention to the check-code-quality results. I'm ignoring them indeed. The code still needs a big refactor and I didn't want to waste time on these.

pawamoy commented 3 years ago

However I'm not sure why the test suite doesn't pass anymore... EDIT: hmmm I think pydantic is also missing from the dev-deps.

bstadlbauer commented 3 years ago

Just just saw the same - adding it locally and testing again

bstadlbauer commented 3 years ago

Works locally - added it here aswell, let's see if CI likes it

bstadlbauer commented 3 years ago

Looks good :-) There is nothing more from my side, so ready to merge

pawamoy commented 3 years ago

Thanks again!