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

Import error on sphinxcontrib.napoleon on Python 3.10.8 #65

Closed
sebastian-correa opened this issue Jan 22, 2023 · 6 comments · Fixed by #69
Closed

Import error on sphinxcontrib.napoleon on Python 3.10.8 #65

sebastian-correa opened this issue Jan 22, 2023 · 6 comments · Fixed by #69
Assignees

Comments

@sebastian-correa
Copy link

Hey! I'm getting this ImportError when running docsig --help:

ImportError: cannot import name 'Callable' from 'collections'

docsig requires sphinxcontrb-napoleon directly, to its latest version of 0.7 which was released in 2018.

As seen in this commit to sphinxcontrib-napoleon, the issue I'm facing was fixed in 2021. I tried copy-pasting that commit into my venv and docsig --help ran correctly.

I see 2 alternatives:

  1. Given that napoleon now comes bundled with sphinx (at sphinx.ext.napoleon), docsig should have a direct requirement on sphinx. sphinx is quite big, though, and you may not want this.
  2. Specify a git dependency [ref] for sphinxcontrib-napoleon, knowing that that repo is not kept up to date.

I can work on a PR to fix this with whatever solution you prefer.

@sebastian-correa
Copy link
Author

sebastian-correa commented Jan 22, 2023

I tried a simple replacement in _function.py (changed import to import sphinx.ext.napoleon as _s) and many tests are failing. I don't have experience with pytest nor docsig, so fixing these seems a tall order.

=========================== short test summary info ============================
FAILED tests/_test.py::test_ignore_no_params[f-e-1-0-4-ret-type-docs-1-sum-n-i] - AssertionError: assert ('E104: return statement documented for None' in '' ...
FAILED tests/_test.py::test_main_sum[f-e-1-0-4-ret-type-docs-1-sum-n-i] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_ignore_no_params[f-no-ret-docs-no-type-n-i] - assert ('def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param par...
FAILED tests/_test.py::test_main_output_negative[f-no-ret-docs-no-type-n-i] - assert 'def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param para...
FAILED tests/_test.py::test_main_str_out[f-no-ret-docs-no-type-n-i] - assert 'def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param para...
FAILED tests/_test.py::test_ignore_kwargs[f-e-1-0-4-ret-type-docs-1-sum-n] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_main_str[f-e-1-0-4-ret-type-docs-1-sum-n] - assert 0 == 1
FAILED tests/_test.py::test_main_output_negative[4RET2] - assert 'E104: return statement documented for None' in ''
FAILED tests/_test.py::test_ignore_no_params[f-no-ret-docs-no-type-n] - assert ('def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param par...
FAILED tests/_test.py::test_main_args[f-e-1-0-4-ret-type-docs-1-sum-n] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_main_args[f-e-1-0-4-ret-type-docs-1-sum-n-i] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_ignore_args[f-e-1-0-4-ret-type-docs-1-sum-n-i] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_ignore_no_params[f-e-1-0-4-ret-type-docs-1-sum-n] - AssertionError: assert ('E104: return statement documented for None' in '' ...
FAILED tests/_test.py::test_main_str_out[4RET1] - assert 'E104: return statement documented for None' in ''
FAILED tests/_test.py::test_main_str_out[f-no-ret-docs-no-type-n] - assert 'def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param para...
FAILED tests/_test.py::test_ignore_kwargs[f-e-1-0-4-ret-type-docs-1-sum-n-i] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_main_output_negative[f-no-ret-docs-no-type-n] - assert 'def function(✓param1, ✓param2, ✓param3)?:\n    """\n    :param para...
FAILED tests/_test.py::test_main_str_out[4RET2] - assert 'E104: return statement documented for None' in ''
FAILED tests/_test.py::test_main_sum[f-e-1-0-4-ret-type-docs-1-sum-n] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_main_output_negative[4RET1] - assert 'E104: return statement documented for None' in ''
FAILED tests/_test.py::test_ignore_args[f-e-1-0-4-ret-type-docs-1-sum-n] - AssertionError: assert 0 == 1
FAILED tests/_test.py::test_main_str[f-e-1-0-4-ret-type-docs-1-sum-n-i] - assert 0 == 1

Ran this on Python 3.8.5.

@jshwi
Copy link
Owner

jshwi commented Jan 22, 2023

Hi @sebastian-correa,
Thank you for spotting this
I welcome a PR if you find a solution
I will also have a look when I get a moment
Cheers

jshwi added a commit that referenced this issue Jan 23, 2023
Signed-off-by: jshwi <stephen@jshwisolutions.com>
@jshwi jshwi self-assigned this Jan 23, 2023
@jshwi jshwi linked a pull request Jan 23, 2023 that will close this issue
jshwi added a commit that referenced this issue Jan 23, 2023
Signed-off-by: jshwi <stephen@jshwisolutions.com>
@jshwi jshwi closed this as completed in #69 Jan 23, 2023
@jshwi
Copy link
Owner

jshwi commented Jan 23, 2023

@sebastian-correa, thanks for your help
sphinxcontrib.napolean appears to be upstream of sphinx.ext.napolean, so moving forward (regardless of the increased package size) sphinx is a necessary component
The failed tests actually failed correctly. The syntax was not correct and only the return type was provided, not the return type and value - those tests have been fixed
I hope this works well for you now, if you come across anything else don't hesitate to open a PR or issue
Cheers!

@sebastian-correa
Copy link
Author

Thanks for the quick reply! When I have time to look into this, I'll notify you here so that we don't do duplicate work. Cool tool, by the way!

@sebastian-correa
Copy link
Author

I don't know what's up with my out-of-order comment. I saw the release; that was blazing fast! I'm sorry I couldn't submit a PR with a fix, but I will continue implementing docsig into our codebase.

@jshwi
Copy link
Owner

jshwi commented Jan 26, 2023

Hi @sebastian-correa,

You helped hurry along this release with the reading up you did on this

I wanted this to work quicker for you so I went ahead with the release, as I knew what the issue was, based on my knowledge of this code

I'll see if I can put together a CONTRIBUTING.md together to make it easier for you to navigate the codebase if you ever want to submit a PR

Also, next time if you find something here, I would be happy for you to make a WIP PR, even if it fails, and we could discuss it

Thanks for the support!

Stephen

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