Docs: ⬆️ Update sphinx + extensions versions #4470

chrisjsewell · 2020-10-19T13:28:04Z

In, particular sphinx v2 -> v3

Also moved sphinxext testing to the official sphinx testing infrastructure

codecov · 2020-10-19T16:35:59Z

Codecov Report

Merging #4470 into develop will decrease coverage by 0.02%.
The diff coverage is 77.78%.

@@             Coverage Diff             @@
##           develop    #4470      +/-   ##
===========================================
- Coverage    79.29%   79.27%   -0.01%     
===========================================
  Files          475      475              
  Lines        34835    34837       +2     
===========================================
- Hits         27618    27614       -4     
- Misses        7217     7223       +6

Flag	Coverage Δ
#django	`73.12% <77.78%> (-0.01%)`	⬇️
#sqlalchemy	`72.34% <77.78%> (-<0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/tools/groups/paths.py	`91.62% <ø> (ø)`
aiida/orm/nodes/data/list.py	`79.27% <50.00%> (-0.97%)`	⬇️
aiida/sphinxext/calcjob.py	`93.34% <66.67%> (-6.66%)`	⬇️
aiida/sphinxext/workchain.py	`93.34% <66.67%> (-6.66%)`	⬇️
aiida/sphinxext/process.py	`95.14% <90.00%> (-0.66%)`	⬇️
aiida/engine/daemon/runner.py	`79.32% <0.00%> (-3.44%)`	⬇️
aiida/transports/plugins/local.py	`82.57% <0.00%> (-0.25%)`	⬇️

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 2174924...446d1a3. Read the comment docs.

chrisjsewell · 2020-10-19T16:44:07Z

@csadorf can you review this, with respect to dependency changes ta. I updated the requirements as mentioned in: #4466 (comment)

chrisjsewell · 2020-10-19T16:59:42Z

@ltalirz and @greschd do you want to check the sphinx extension side of this PR

~~I will also have a look at #3300 and #3689 in a bit and see if they can be closed~~ done

(I'm tempted to change the dropdown implementation to https://sphinx-panels.readthedocs.io/en/latest/#dropdown-usage, but I don't know if I'll have time right now)

chrisjsewell · 2020-10-19T20:44:45Z

aiida/sphinxext/calcjob.py

@@ -27,8 +29,7 @@ class AiidaCalcJobDocumenter(AiidaProcessDocumenter):

    @classmethod
    def can_document_member(cls, member, membername, isattr, parent):
-        from aiida.engine import CalcJob
-        return issubclass(cls, CalcJob)


Found your error @ltalirz 😄

Great, thanks a lot Chris!

greschd · 2020-10-19T20:53:34Z

Also moved sphinxext testing to the official sphinx testing infrastructure

Is there an easy way to manually check the HTML output with the official testing infra? That was the reason for that Makefile.

chrisjsewell · 2020-10-19T20:56:20Z

Is there an easy way to manually check the HTML output with the official testing infra? That was the reason for that Makefile.

I generally use beautifulsoup + pytest-regressions: https://github.com/executablebooks/MyST-Parser/blob/750ea18db6e7eccc147e3bf45921ac4eb6715ab5/tests/test_sphinx/conftest.py#L80

greschd · 2020-10-19T21:16:46Z

Not sure if that answers my question (maybe I'm just being dense): Can I create the HTML output of the "test documentations" without changing the test code?

chrisjsewell · 2020-10-19T21:18:52Z

Gah-damn, now that #3689 is fixed, you need to reference the processes with their new object types e.g. :py:process: or :process:, not :py:class: or :class:

tests/sphinxext/test_workchain.py

ltalirz

thanks a lot @chrisjsewell !

Just a couple of comments & minor questions.

@greschd Please also let us know whether you're happy with this solution.

ltalirz · 2020-10-19T21:09:59Z

aiida/orm/nodes/data/list.py

@@ -77,15 +77,18 @@ def remove(self, value):
        del self[value]

    def pop(self, **kwargs):  # pylint: disable=arguments-differ
+        """Remove and return item at index (default last)."""


That function does not seem to return anything or am I wrong?

Hmm, yeh good point it should actually return; thats a bug in the code I would say.

Its a bit out of scope of this PR, so perhaps we should fix in another small PR.
(I only actually made this change to fix a warning by sphinx about the inherited docstring)

Up to you - in any case, the docstring should be consistent and if you prefer not to fix it here, we should open an issue

ok I've fixed it here then 446d1a3

ltalirz · 2020-10-19T21:12:35Z

aiida/tools/groups/paths.py

@@ -257,7 +257,7 @@ def walk_nodes(

        :param filters: filters to apply to the node query
        :param node_class: return only nodes of a certain class (or list of classes)
-        :param int batch_size: The size of the batches to ask the backend to batch results in subcollections.
+        :param query_batch: The size of the batches to ask the backend to batch results in subcollections.


batch_size might actually be the better name; anyhow it's fine to adapt the docs instead.

yeh it probably should be, but thats outside the scope of this PR

docs/source/conf.py

tests/sphinxext/sources/workchain/conf.py

tests/sphinxext/sources/workchain_broken/conf.py

tests/sphinxext/reference_results/workchain.xml

ltalirz · 2020-10-19T21:22:36Z

Sorry, that should have been "request changes" ;-)

ltalirz · 2020-10-19T21:23:59Z

Is there a links to the docs that we can check?

chrisjsewell · 2020-10-19T21:26:34Z

you just click on details:

then artifacts/html/index.html

https://1443-77234579-gh.circle-artifacts.com/0/html/index.html

chrisjsewell · 2020-10-19T23:57:19Z

@ltalirz @greschd I've rolled back (i.e. commented out) the autodoc writer, because it doesn't work with referencing. From the current implementation, I'm pretty sure that has never worked.

As I mention in #3300 (comment), I think the whole extension needs a bit of a re-write to fix this.
But I'm definitely not doing it in this PR, since the point was really only to upgrade to sphinx 3 and now I've already wasted enough time messing around with it 😒

csadorf

@csadorf can you review this, with respect to dependency changes ta. I updated the requirements as mentioned in: #4466 (comment)

I have one question, but generally fine w.r.t. the updates to the dependency specification.

setup.json

greschd · 2020-10-20T06:45:45Z

@ltalirz @greschd I've rolled back (i.e. commented out) the autodoc writer, because it doesn't work with referencing. From the current implementation, I'm pretty sure that has never worked.

Can you revert this? There are existing plugins using automodule - and I think referencing being broken isn't as bad as regressing to a "plain" Sphinx class.

chrisjsewell · 2020-10-20T08:30:26Z

Can you revert this? There are existing plugins using automodule - and I think referencing being broken isn't as bad as regressing to a "plain" Sphinx class.

@greschd Unfortunately no. When I say revert, I mean revert from earlier in this PR. automodule has already regressed to plain sphinx classes (for about a year!) and turning it on breaks 116 references in the aiida-core documentation, with no fix until the whole extension is basically re-wrote.

greschd · 2020-10-20T08:43:52Z

Ah yeah now I remember, that was #3689. I'm not sure referencing ever worked (although I think I was following some guide when writing the directive). Out of curiosity, why does the extension need to be rewritten for referencing to work?

chrisjsewell · 2020-10-20T08:58:09Z

Basically we need to "integrate" it into the "py" domain, a bit like sphinx/domains/python::PyClasslike does, such that it also provides an XRefRole and stores an object in the py domain.
The actual creation of the doctree is generally fine though, its just the stuff around it.

chrisjsewell added 2 commits October 19, 2020 15:26

Docs: ⬆️ Updata sphinx version + extensions

f53b3e9

Also moved sphinxext testing to the official sphinx testing infrastructure

Merge branch 'develop' into docs/update-versions

6c9578e

chrisjsewell requested a review from ramirezfranciscof October 19, 2020 13:29

chrisjsewell changed the title ~~Docs: ⬆️ Updata sphinx version + extensions~~ Docs: ⬆️ Update sphinx version + extensions Oct 19, 2020

chrisjsewell added 4 commits October 19, 2020 16:06

update

e6a76c4

Fix tests

648f68d

Merge branch 'develop' into docs/update-versions

772b40f

Update setup.json

cd37991

chrisjsewell requested a review from csadorf October 19, 2020 16:38

chrisjsewell requested review from ltalirz and greschd October 19, 2020 16:56

chrisjsewell added 2 commits October 19, 2020 22:40

Fix autodoc bug

0b38ff1

Merge branch 'develop' into docs/update-versions

1ab9af8

chrisjsewell commented Oct 19, 2020

View reviewed changes

fix test

29954d4

chrisjsewell commented Oct 19, 2020

View reviewed changes

tests/sphinxext/test_workchain.py Show resolved Hide resolved

ltalirz previously approved these changes Oct 19, 2020

View reviewed changes

chrisjsewell mentioned this pull request Oct 19, 2020

Documenting WorkChain and CalcJob plugins #3300

Closed

roll-back autodoc

1b3d522

chrisjsewell dismissed ltalirz’s stale review via 1b3d522 October 19, 2020 23:51

chrisjsewell requested a review from ltalirz October 19, 2020 23:57

fix List.pop

446d1a3

csadorf approved these changes Oct 20, 2020

View reviewed changes

setup.json Show resolved Hide resolved

chrisjsewell changed the title ~~Docs: ⬆️ Update sphinx version + extensions~~ Docs: ⬆️ Update sphinx + extensions versions Oct 20, 2020

chrisjsewell merged commit eed1917 into aiidateam:develop Oct 20, 2020

chrisjsewell deleted the docs/update-versions branch October 20, 2020 08:37

chrisjsewell mentioned this pull request Oct 20, 2020

Sphinx directives are no longer hooked into sphinx.ext.autodoc #3689

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs: ⬆️ Update sphinx + extensions versions #4470

Docs: ⬆️ Update sphinx + extensions versions #4470

chrisjsewell commented Oct 19, 2020 •

edited

Loading

codecov bot commented Oct 19, 2020 •

edited

Loading

chrisjsewell commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020 •

edited

Loading

chrisjsewell Oct 19, 2020

ltalirz Oct 19, 2020

greschd commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020

greschd commented Oct 19, 2020 •

edited

Loading

chrisjsewell commented Oct 19, 2020

ltalirz left a comment

ltalirz Oct 19, 2020

chrisjsewell Oct 19, 2020

ltalirz Oct 19, 2020

chrisjsewell Oct 20, 2020

ltalirz Oct 19, 2020

chrisjsewell Oct 19, 2020

ltalirz commented Oct 19, 2020

ltalirz commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020 •

edited

Loading

chrisjsewell commented Oct 19, 2020

csadorf left a comment

greschd commented Oct 20, 2020

chrisjsewell commented Oct 20, 2020

greschd commented Oct 20, 2020

chrisjsewell commented Oct 20, 2020

Docs: ⬆️ Update sphinx + extensions versions #4470

Docs: ⬆️ Update sphinx + extensions versions #4470

Conversation

chrisjsewell commented Oct 19, 2020 • edited Loading

codecov bot commented Oct 19, 2020 • edited Loading

Codecov Report

chrisjsewell commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greschd commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020

greschd commented Oct 19, 2020 • edited Loading

chrisjsewell commented Oct 19, 2020

ltalirz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltalirz commented Oct 19, 2020

ltalirz commented Oct 19, 2020

chrisjsewell commented Oct 19, 2020 • edited Loading

chrisjsewell commented Oct 19, 2020

csadorf left a comment

Choose a reason for hiding this comment

greschd commented Oct 20, 2020

chrisjsewell commented Oct 20, 2020

greschd commented Oct 20, 2020

chrisjsewell commented Oct 20, 2020

chrisjsewell commented Oct 19, 2020 •

edited

Loading

codecov bot commented Oct 19, 2020 •

edited

Loading

chrisjsewell commented Oct 19, 2020 •

edited

Loading

greschd commented Oct 19, 2020 •

edited

Loading

chrisjsewell commented Oct 19, 2020 •

edited

Loading