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

Improve documentation targets. #1725

Merged
merged 2 commits into from
Mar 25, 2022
Merged

Improve documentation targets. #1725

merged 2 commits into from
Mar 25, 2022

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Mar 23, 2022

  • Now they set the required environment variables so that they can be
    built without special environment manipulations.
  • Add NRN_ENABLE_DOCS option, if it's enabled then check that
    dependencies are available as part of the CMake configuration step.
  • Somewhat unrelated: make the modlunit tests into CTest tests.

Closes #1652, closes #1677.

TODO:

@nrnhines
Copy link
Member

This seems to be good place to update nrn/docs/cmake_doc/option after NRN_ENABLE_TESTS with an explanation of NRN_ENABLE_DOCS and also in nrn/docs/README.md

@olupton
Copy link
Collaborator Author

olupton commented Mar 24, 2022

I added some documentation.

I am still trying to adjust the CMake/build rules to make it behave sensibly. Right now then ninja sphinx always re-does the (very slow) notebook conversion.

@alexsavulescu
Copy link
Member

I added some documentation.

I am still trying to adjust the CMake/build rules to make it behave sensibly. Right now then ninja sphinx always re-does the (very slow) notebook conversion.

for reference: https://github.com/neuronsimulator/nrn/tree/master/docs#faster-local-iterations

@olupton
Copy link
Collaborator Author

olupton commented Mar 24, 2022

Indeed. I was trying to rework that a bit to use standard build dependencies, but I may have to give up on that.

@alexsavulescu
Copy link
Member

Indeed. I was trying to rework that a bit to use standard build dependencies, but I may have to give up on that.

Yeah, put in quite a lot of thinking to arrive to what we have now.

@olupton
Copy link
Collaborator Author

olupton commented Mar 24, 2022

I see

job execution unauthorized: This job has been blocked because resource-class:arm.medium is not available on your plan.

in circleci.

@alexsavulescu
Copy link
Member

Out of credits ...

@olupton olupton closed this Mar 24, 2022
@olupton olupton reopened this Mar 24, 2022
@alexsavulescu
Copy link
Member

RTD fails:

-- Could NOT find # 3.1.0 is buggy (missing: # 3.1.0 IS BUGGY_LOCATION _# 3.1.0 IS BUGGY_VERSION_MATCH) 
CMake Error at cmake/modules/FindPythonModule.cmake:66 (message):
  Missing python module # 3.1.0 is buggy
Call Stack (most recent call first):
  CMakeLists.txt:533 (nrn_find_python_module)

@alexsavulescu
Copy link
Member

also in Docs CI

-- Could NOT find jinja2!=3.1.0  # 3.1.0 is buggy (missing: JINJA2!=3.1.0  # 3.1.0 IS BUGGY_LOCATION _JINJA2!=3.1.0  # 3.1.0 IS BUGGY_VERSION_MATCH) 
CMake Error at cmake/modules/FindPythonModule.cmake:66 (message):
  Missing python module jinja2!=3.1.0 # 3.1.0 is buggy
Call Stack (most recent call first):
  CMakeLists.txt:533 (nrn_find_python_module)

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.34%. Comparing base (e7633ff) to head (30872e3).
Report is 1814 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1725      +/-   ##
==========================================
+ Coverage   45.30%   45.34%   +0.03%     
==========================================
  Files         551      551              
  Lines      111219   111219              
==========================================
+ Hits        50387    50431      +44     
+ Misses      60832    60788      -44     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@olupton olupton force-pushed the olupton/docs branch 2 times, most recently from 1284c20 to 7224ee1 Compare March 25, 2022 06:12
@olupton
Copy link
Collaborator Author

olupton commented Mar 25, 2022

@alexsavulescu
Copy link
Member

Copy link
Member

@alexsavulescu alexsavulescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@alexsavulescu
Copy link
Member

@ramcdougal would you mind giving this a shot on your VM ?

- Now they set the required environment variables so that they can be
  built without special environment manipulations.
- Add NRN_ENABLE_DOCS option, if it's enabled then check that
  dependencies are available as part of the CMake configuration step.
- Add NRN_ENABLE_DOCS_WITH_EXTERNAL_INSTALLATION option to support the
  legacy/hybrid workflow where we build documentation using some other,
  unknown, external installation of NEURON instead of the build of
  NEURON that is currently being configured.
- Somewhat unrelated: make the modlunit tests into CTest tests.

Closes #1652, closes #1677.
@ramcdougal
Copy link
Member

@alexsavulescu This looks promising, unfortunately any tests I do would be artificial as #1699 (I think) fixed my issues which was due to having multiple Pythons available and the make sphinx somehow finding the wrong one... after #1699 docs building just worked for me across the board.

@alexsavulescu alexsavulescu merged commit 30b9eac into master Mar 25, 2022
@alexsavulescu alexsavulescu deleted the olupton/docs branch March 27, 2022 13:48
@alexsavulescu alexsavulescu mentioned this pull request Jun 29, 2022
19 tasks
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.

DOCS: add NRN_ENABLE_DOCS=ON Documentation targets do not set all required environment variables.
5 participants