Implement support for interfaces in chpldoc

[lydia-duncan]

Involves:

• updating the Chapel sphinx domain to recognize the keyword and output it appropriately into the documentation (similar to how classes and records are handled, but with a little more nuance).
• updating the ast node handling in the docs pass to output the keyword
• adding chpldoc tests of interfaces and implements clauses
• other things?

[Venkatavaradan-R]

I'd like to work on this!
Currently going through some material getting acquainted with sphinx and rst.
Is there any documentation specific to the chapel project that I should be aware of? Any help appreciated!

[lydia-duncan]

Oh cool! This is more from the feature side of things but may still be useful to you. Definitely let me know if you have any questions

[Venkatavaradan-R]

Hi @lydia-duncan

I've been getting acquainted with the chpldoc and the sphinxcontrib-chapeldomain codebase.

updating the Chapel sphinx domain to recognize the keyword and output it appropriately into the documentation (similar to how classes and records are handled, but with a little more nuance).

Could you point me to where I can find how the classes and records are handled? I'm assuming it is somewhere here.

updating the ast node handling in the docs pass to output the keyword

Could you elaborate as to what I need to do here as well. Pointing me to some code will definitely help!
Thanks in advance!

[lydia-duncan]

updating the Chapel sphinx domain to recognize the keyword and output it appropriately into the documentation (similar to how classes and records are handled, but with a little more nuance).

Could you point me to where I can find how the classes and records are handled? I'm assuming it is somewhere here.

You'll probably use something this from that file as part of your implementation, and make a version of this for interfaces. I think other changes will be best found by searching for references to those structures.

updating the ast node handling in the docs pass to output the keyword

Could you elaborate as to what I need to do here as well. Pointing me to some code will definitely help!

Sure! The main Chapel compiler stuff supporting chpldoc happens in this file or in the printDocs method of individual AST nodes (e.g. FnSymbol's printDocs method). For the specific AST node handling of interfaces, @vasslitvinov should be able to give pointers - I haven't looked through that code yet.

[Venkatavaradan-R]

Hi Lydia!
Thanks for the links to the code, It helped quite a bit.

After a lot of time spent (mostly) understanding what's happening in sphinxcontrib-chpldomain, I think I've understood what exactly needs to be done.

the chpldomain needs to be updated in order to be able to output documentation for interfaces in this manner. Am I correct in my understanding?

If so, that brings me back to the code currently available for classes/records. I'm not able to figure out what exactly is going to be different from the code available, other than having to add interface in this line of code and updating the domain by adding 'interface': ChapelClassObject, one line below this. Would this not be enough?

[lydia-duncan]

the chpldomain needs to be updated in order to be able to output documentation for interfaces in this manner. Am I correct in my understanding?

Partially! We definitely will want to be able to reference the interfaces that have been defined and used, probably by making a :implements: tag that can be attached to a record or class definition like how the :arg: and :return: tags are available for functions and by making a :interface: role like classes and records have so that a mention will link to the definition point. But I think we'll also want to see these kinds of definitions but for interfaces so that there is something to link to.

If so, that brings me back to the code currently available for classes/records. I'm not able to figure out what exactly is going to be different from the code available, other than having to add interface in this line of code and updating the domain by adding 'interface': ChapelClassObject, one line below this. Would this not be enough?

That looks like places you'll want to update, yeah. I think the "role" section below the last two will also need to be updated like I mentioned earlier in this comment, and then you'll need to look elsewhere for how to add the :implements: tag. After that point, I think writing some test cases in the Sphinx domain will be your best bet for assuring yourself that it all works.

[Venkatavaradan-R]

To Dos:

• Interface Definition (Generic definition like this. Find it here on line 133. Updates need to be made here and here as well.)
• Interface Role (:interface: that links to the above mentioned definition. Codebase link (need to find where this is defined). Update here to allow cross referencing.)
• Implements tag (similar to :arg: and :return:, defined here, documented here under Info field lists)
• write tests to check if everything works.
• updating the ast node handling in the docs pass to output the keyword (don't quite understand what this is yet, but will get to it)
• update the README at the root of the project with examples.

[Venkatavaradan-R]

So when a definition is made in the references.rst file, for example, the class directive on line 133, and the necessary changes are made here and here(for the definition itself) and here to enable cross-referencing for the role, the interface role will link directly to the definition in references.rst?

Also, could you check if I'm heading in the right direction wrt the todos I've posted above?
Thanks!

[lydia-duncan]

the interface role will link directly to the definition in references.rst

I believe so, but to be absolutely clear, I would expect you to have to write :interface:MyInterfaceName to refer to the interface named MyInterfaceName.

Those are very good TODOs - the last one will need to be merged once the rest are merged into the Sphinx domain repository and a new release is pushed (which I control and won't require as much pomp as the chapel-lang/chapel repository does). I believe there are ways to test it prior to an official Sphinx domain release, to ensure we don't have to push multiple releases to get something that chpldoc itself fully uses.

[Venkatavaradan-R]

I believe so, but to be absolutely clear, I would expect you to have to write :interface:MyInterfaceName to refer to the interface named MyInterfaceName.

I see!
I've seen this kind of logic being applied here, but I'm unable to pinpoint where this logic is defined. Could you help me with that?
I can see that the logic for Info Fields Lists or tags has been implemented here.

Also, tags are not specific to any single directive like classes or records right?

the last one will need to be merged once the rest are merged into the Sphinx domain repository and a new release is pushed

Sounds good. I'll focus on the rest for now.

I believe there are ways to test it prior to an official Sphinx domain release, to ensure we don't have to push multiple releases to get something that chpldoc itself fully uses.

I was curious about this as well. How would this work?

[lydia-duncan]

but I'm unable to pinpoint where this logic is defined

I think that the logic is at least partially handled by Sphinx itself, see Sphinx's documentation on roles, so probably what you need to do is just make sure it is listed as a role and Sphinx will handle the rest? I'm not completely certain but that seems likely.

I can see that the logic for Info Fields Lists or tags has been implemented here.

That looks right to me.

Also, tags are not specific to any single directive like classes or records right?

Huh, looking at the implementation and testing it myself I guess you are right? You might be able to add the :implements: tag specifically for just classes and records because those are a specific subclass of ChapelObject, but it does seem like the rest are only limited due to convention rather than the Sphinx domain explicitly excluding them. Interesting - I would guess it has something to do with functions being split between ChapelModuleLevel and ChapelClassMember. There may be a way to handle that in a more limited manner without duplicating code or something, but that seems like a separate issue (maybe worth opening an issue on?)

I believe there are ways to test it prior to an official Sphinx domain release, to ensure we don't have to push multiple releases to get something that chpldoc itself fully uses.

I was curious about this as well. How would this work?

If my memory serves, it involves modifying the sphinxcontrib-chapeldomain line in $CHPL_HOME/third-party/chpl-venv/chpldoc-requirements.txt. I think you can make that a relative path to where your repository is checked out (but be sure not to commit that change!)

[Venkatavaradan-R]

just make sure it is listed as a role and Sphinx will handle the rest?

I'm testing this out now. I also found some articles and material online about custom sphinx roles (ref). These are the roles defined in the chapel domain. To be honest, I'm not quite sure what is happening with roles.

There may be a way to handle that in a more limited manner without duplicating code or something, but that seems like a separate issue (maybe worth opening an issue on?)

Looks like it. I feel the init.py file might benefit from some refactoring on the whole (comments are from 2010 and docs/comments can be a bit more descriptive).

what does retann stand for? (ref)

[Venkatavaradan-R]

Is there any specific necessity to run tests on sphinxcontrib-chpldomain using py2.7 as well?
I get the following error message when I run tox.

ERROR: Could not find a version that satisfies the requirement Sphinx==3.2.1 (from -r requirements.txt (line 3)) (from versions: 0.1.61611, 0.1.61798, 0.1.61843, 0.1.61945, 0.1.61950, 0.2, 0.3, 0.4, 0.4.1, 0.4.2, 0.4.3, 0.5, 0.5.1, 0.5.2b1, 0.5.2, 0.6b1, 0.6, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.6.7, 1.0b1, 1.0b2, 1.0, 1.0.1, 1.0.2, 1.0.3, 1.0.4, 1.0.5, 1.0.6, 1.0.7, 1.0.8, 1.1, 1.1.1, 1.1.2, 1.1.3, 1.2b1, 1.2b2, 1.2b3, 1.2, 1.2.1, 1.2.2, 1.2.3, 1.3b1, 1.3b2, 1.3b3, 1.3, 1.3.1, 1.3.2, 1.3.3, 1.3.4, 1.3.5, 1.3.6, 1.4a1, 1.4b1, 1.4, 1.4.1, 1.4.2, 1.4.3, 1.4.4, 1.4.5, 1.4.6, 1.4.7, 1.4.8, 1.4.9, 1.5a1, 1.5a2, 1.5b1, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.5.4, 1.5.5, 1.5.6, 1.6b1, 1.6b2, 1.6b3, 1.6.1, 1.6.2, 1.6.3, 1.6.4, 1.6.5, 1.6.6, 1.6.7, 1.7.0b1, 1.7.0b2, 1.7.0, 1.7.1, 1.7.2, 1.7.3, 1.7.4, 1.7.5, 1.7.6, 1.7.7, 1.7.8, 1.7.9, 1.8.0b1, 1.8.0, 1.8.1, 1.8.2, 1.8.3, 1.8.4, 1.8.5)

Its been specified here.

Also, I think it may be better to just change the envlist variable to py3 or py from py34. I'm running py39 on my local system. I don't mind changing the file to match my Python version when testing, but just wondering if it's actually necessary.

[lydia-duncan]

what does retann stand for? (ref)

Huh. I'm not sure. My best guess is currently that it should be "return" but was a typo, given what else is in the list of groups.

Is there any specific necessity to run tests on sphinxcontrib-chpldomain using py2.7 as well?

No. We expect Python 2 to stop being available in newer system, and earlier versions of Sphinx aren't compatible with a newer Python 3 than 3.4 if I remember right (which is also deprecated if I remember right).

Also, I think it may be better to just change the envlist variable to py3 or py from py34. I'm running py39 on my local system. I don't mind changing the file to match my Python version when testing, but just wondering if it's actually necessary.

Huh, I hadn't noticed that was still like that. My use of tox has been through running the release script but that script uses a later version of Python as well, maybe it was working for me because that version was overriding the version in that file? We probably need to adjust our scripts to better test that style of execution.

[Venkatavaradan-R]

Hey @lydia-duncan !

Just a quick update.

what does retann stand for?

After searching around a bit, I'm quite positive retann here means return annotation (specifying the return type of the function).
Is ref a datatype in chapel? (I'm assuming it means reference, but what does it mean? A pointer perhaps?). It has been used regularly in the tests.

My use of tox has been through running the release script but that script uses a later version of Python as well, maybe it was working for me because that version was overriding the version in that file? We probably need to adjust our scripts to better test that style of execution.

Just to confirm, you mean this file right?
It does seem to be overriding the version in the tox file. I'll read up on this and open a new issue and PR once I'm finished with this issue.

I've written a few test cases to check if the changes I mentioned in the above to-do list are enough, but some of them are failing. I'll keep you posted.

[lydia-duncan]

Hey @lydia-duncan !

Just a quick update.

what does retann stand for?

After searching around a bit, I'm quite positive retann here means return annotation (specifying the return type of the function).

Oh, neat!

Is ref a datatype in chapel? (I'm assuming it means reference, but what does it mean? A pointer perhaps?). It has been used regularly in the tests.

ref is not a type on its own, it is an argument intent and return intent. There's been talk of making it a way to declare a variable or field, similar to var or const, but so far as I know it has not be implemented yet.

My use of tox has been through running the release script but that script uses a later version of Python as well, maybe it was working for me because that version was overriding the version in that file? We probably need to adjust our scripts to better test that style of execution.

Just to confirm, you mean this file right?
It does seem to be overriding the version in the tox file. I'll read up on this and open a new issue and PR once I'm finished with this issue.

I was referring to this file, but that one also uses tox and overrides its settings.

I've written a few test cases to check if the changes I mentioned in the above to-do list are enough, but some of them are failing. I'll keep you posted.

Awesome, thanks :)