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

Add sphinx v2.4 as run dependency #399

Conversation

winksaville
Copy link
Contributor

With conda 4.8.3 on my Arch Linux system the default sphinx version is
3.1. I get the following error when running ./build-docs.sh:

$ ./build-docs.sh
Running Sphinx v3.1.1

Exception occurred:
File "/opt/anaconda/envs/cqgui-master3/lib/python3.7/site-packages/cadquery/cq_directive.py", line 93, in setup
app.add_directive("cq_plot", cq_directive, True, (0, 2, 0), **options)
TypeError: add_directive() got an unexpected keyword argument 'height'
The full traceback has been saved in /tmp/sphinx-err-euymmkgl.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at https://github.com/sphinx-doc/sphinx/issues. Thanks!

With sphinx 2.4 I was able to build the docs.

Fixes #398

With conda 4.8.3 on my Arch Linux system the default sphinx version is
3.1. I get the following error when running `./build-docs.sh`:

$ ./build-docs.sh
Running Sphinx v3.1.1

 Exception occurred:
   File "/opt/anaconda/envs/cqgui-master3/lib/python3.7/site-packages/cadquery/cq_directive.py", line 93, in setup
     app.add_directive("cq_plot", cq_directive, True, (0, 2, 0), **options)
 TypeError: add_directive() got an unexpected keyword argument 'height'
 The full traceback has been saved in /tmp/sphinx-err-euymmkgl.log, if you want to report the issue to the developers.
 Please also report this if it was a user error, so that a better error message can be provided next time.
 A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!

With sphinx 2.4 I was able to build the docs.

Fixes CadQuery#398
@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #399 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #399   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files          19       19           
  Lines        5004     5004           
  Branches      514      514           
=======================================
  Hits         4677     4677           
  Misses        205      205           
  Partials      122      122           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bacd459...14cec84. Read the comment docs.

@adam-urbanczyk
Copy link
Member

I'm afraid that I cannot merge this - sphinx is not a dependency of the conda package and certainly not a runtime dependency of CQ. The dev env is specified here: https://github.com/CadQuery/cadquery/blob/master/environment.yml

@winksaville
Copy link
Contributor Author

I'm afraid that I cannot merge this - sphinx is not a dependency of the conda package and certainly not a runtime dependency of CQ. The dev env is specified here: https://github.com/CadQuery/cadquery/blob/master/environment.yml

OK, but I'd like to suggest there needs to be some additional documentation in the Contributing code section of README.md between these two items:

  • Fork the CadQuery repository, clone your fork and create a new branch to start working on your changes
  • Start with the tests! How should CadQuery behave after your changes? Make sure to add some tests to the test suite to ensure proper behavior

My suggestion is something like:

  • Fork the CadQuery repository, clone your fork and create a new branch to start working on your changes
  • Create a conda development environment with something like: conda env create -n cq-dev -f environment.yml
  • Activate the new conda enviornment: conda activate cq-dev
  • Install the master branch of cq-editor, if desired: conda install -c cadquery -c conda-forge cq-editor=master
  • Before making any changes verify that the current tests pass.
    • Run pytest from the root of your cadquery clone, the result should be something like:
      ============================ 215 passed, 57 warnings in 13.95s ============================
  • Start with the tests! How should CadQuery behave after your changes? Make sure to add some tests to the test suite to ensure proper behavior

I'll be glad to make a pull request if you agree something like the above would be helpful. Of course you may disagree, which is fine too.

@adam-urbanczyk
Copy link
Member

If you want make a PR that extends the contributing section, I'm definitely for merging it.

@winksaville winksaville deleted the Add-sphinx-v2.4-as-run-dependency branch August 8, 2020 14:24
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 this pull request may close these issues.

Sphinx version 2.4 needed to build the docs
2 participants