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

[JOSS Review] Documentation enhancements #319

Closed
10 of 11 tasks
blsqr opened this issue May 18, 2022 · 15 comments
Closed
10 of 11 tasks

[JOSS Review] Documentation enhancements #319

blsqr opened this issue May 18, 2022 · 15 comments

Comments

@blsqr
Copy link

blsqr commented May 18, 2022

Hi again @rogersamso, @enekomartinmartinez, and co-authors 👋

This issue is part of the PySD review at JOSS. Following the review checklist, I'd like to make a few remarks regarding the PySD documentation.

First things first: The documentation is well written and very helpful for getting started with PySD and running some example models. With resources like the cookbook linked, I imagine this to be a good starting point for using PySD.
I also like the developer-focussed documentation that details program structure and philosophy!

For publication in JOSS, the following points need to be addressed, though:

  • Statement of need: This is well-explained in the software paper, but is (to my taste) a bit on the short side in the docs. I'd suggest to keep the first two sentences on the docs start page (because they are very crisp), but add another paragraph or so that offers a bit more insight into the motivation behind PySD and the possibilities it offers.
  • Functionality documentation: While you do include bits and pieces of API documentation in appropriate places, the User Functions Reference page does not appear to document PySD's full public API, correct? While I like that it highlights some important functions, I was not able to find a reference for other noteworthy structures like the Model class, it's run method etc.. There should be a page containing the full public API reference.

In addition, I'd have the following thoughts and suggestions regarding the documentation (all optional):

  • The start page currently shows a two-level listing of all pages; this is essentially redundant with the sidebar navigation. I would propose to remove the TOC from the start page and instead focus on more descriptive page titles, such that readers can use the sidebar navigation more efficiently. This would also make the content at the bottom of the start page more discoverable.
  • Consider renaming "Basic Usage" page to something like "Getting started" to more clearly direct new users to this page.
  • Move the "Supported Functions" section (currently on bottom of "Basic Usage" page) to a more appropriate place. To me, that table seems misplaced there; perhaps a separate page that focusses on "Loading vensim and xmile models" would be better?
    • That page could also highlight potential limitations or difficulties when importing these models.
    • Perhaps there is enough overlap with the "Advanced Usage" page to address it all there?
    • Should there also be a table of supported Stella functions? 🤔
  • Make the "About the project" page more visible; I think that's very interesting but it's a bit hidden in the developer docs. This could easily fit on the root level of the docs, I think.
  • The "Contributing to PySD" page seems to be mostly about development guidelines. Given that there already is a remark about contributing to PySD on the start page, it may be clearer to rename this page to something like "Development guidelines"?
  • Consider deploying more than just master/latest to readthedocs.

If there's something unclear in what I wrote above or if I can be of help with the necessary changes, please let me know.

@enekomartinmartinez
Copy link
Collaborator

@blsqr Thanks for those suggestions :)
I will implement them and mark the checkboxes when done.

@JamesPHoughton could you take charge of the deployment of more versions than master/latest in RTD?

@enekomartinmartinez
Copy link
Collaborator

@blsqr I have been working on the PR #312. We have this PR almost ready for merge which includes part of the future roadmap of the paper (splitting parsing from building and Abstract Model representation, it doesn't include the numpy array based backend). We will soon modify the paper slightly to keep it updated.

We are working now on finishing the documentation of that branch and we will include your suggestion. As soon everything is ready I will let you know.

Thanks :)

@JamesPHoughton
Copy link
Collaborator

JamesPHoughton commented May 18, 2022

Consider deploying more than just master/latest to readthedocs

Happy to look at it, can you help me understand more about what this means? Would we want to have a separate version for 2.0 vs 3.0?

@blsqr
Copy link
Author

blsqr commented May 19, 2022

… can you help me understand more about what this means?

@JamesPHoughton ReadTheDocs offers to build Versioned Documentation for tags, such that you can select between the docs of version 2.0, 2.2.4, … via a selector like this, including the new stable tag which points to the latest release (not commit like latest):

Screenshot 2022-05-18 at 23 03 16

Whether you want to do that is another question, I'd say. The main advantage of versioned documentation may be of an archival nature: it allows users to refer to a certain version and have the corresponding docs available on RTD. This may also come in handy when publishing PySD models that were created with a specific version.
If, however, you'd want to nudge users to always working with the latest PySD version (or even stay on HEAD), I think it's understandable to not differentiate versions.

Regarding implementation: In the RTD admin interface you might want to check the Advanced Settings and make sure the "Single Version" setting is not checked. RTD typically sets up a webhook automatically upon project import, but you may have to do this manually, if it does not work. After the webhook is set up, each new tag that matches a semantic version number will trigger a build.

Also note that you can manually edit which versions will be shown publicly. I believe you can also manually trigger builds for older tags, if needed.

@enekomartinmartinez
Copy link
Collaborator

Hi @blsqr ,
I think I have finished implementing your suggestions in the documentation. They have been very helpful to make it cleaner and more useful. If you think some other improvements needs to be done, please, let me know so I can include them before merging #312

@blsqr
Copy link
Author

blsqr commented May 20, 2022

@enekomartinmartinez @rogersamso Having looked at the PR build of the docs for #312, I think it looks great and these are valuable changes. I'm happy you found my suggestions helpful.

There are no further suggestions from my side regarding #312. Whether you keep this issue open (for tracking the changes to RTD deployment) or close it is up to you.

@rogersamso
Copy link
Contributor

rogersamso commented May 20, 2022

Hi @blsqr , I think I have finished implementing your suggestions in the documentation. They have been very helpful to make it cleaner and more useful. If you think some other improvements needs to be done, please, let me know so I can include them before merging #312

Thanks @enekomartinmartinez

@blsqr
Copy link
Author

blsqr commented May 24, 2022

Some minor things I stumbled upon in setup.py:

  • The license field says LICENSE.txt but the actual license file has no .txt ending
  • You still state "Development status: Alpha" as a classifier – why is that? To me, PySD seems well beyond alpha status.

The latter also shows up in the Docs:

Screenshot 2022-05-24 at 09 39 23


Another suggestion regarding the developer documentation: I think it would profit from consolidating the information about locally running tests in one place; currently, they are a bit spread out and partly contradictory regarding the dependencies (compare tests/README.md with tests/requirements.txt). It's nothing unsurmountable, but it would perhaps smooth out the process a bit to have all steps in one place:

  • clone the repo with the --recursive flag
  • potentially create or enter a virtual environment
  • install test dependencies using pip install -r tests/requirements.txt
  • run tests using make tests or using python -m pytest -v tests/

In my mind, this could be the Development Guidelines doc page (in the test suite section) or the tests/README.md file, with one referring to the other.

@enekomartinmartinez
Copy link
Collaborator

Hi @blsqr,
Thank you for your suggestions here and on other issues, they are very useful. I will implement them soon :)
I have doubts about when upgrading the release status from alpha to beta.
If we understand by being feature complete supporting all the Vensim and Xmile functions, we still are not as some are missing. However, PySD is complete in the sense that most models could be translated to Python and run, including the user interaction features.
If you think so, I would be happy to upgrade the status to beta. Let me know what you think about this.

@blsqr
Copy link
Author

blsqr commented May 24, 2022

If we understand by being feature complete supporting all the Vensim and Xmile functions …

Fair enough, I didn't think about that! But maybe that raises an interesting point: the alpha/beta/… status may not be that informative overall, because it's unclear what it relates to for PySD. In my perception, PySD's public interface looks rather stable and it is feature complete regarding many intended use cases. For me that would already warrant a "release" or "production" status, even if there are still some further features and translation functions to be implemented.

At the same time, I see your point: It should be clear that not all Vensim and Xmile functions are available yet. However, would that not be better communicated by explicitly having something like a comparison table that shows which functions are available and which still need to be implemented? An alpha or beta status does not really convey that, in my opinion.

Either way: I think you have to feel happy with the release status and I can understand if you'd want to keep it as alpha or beta 👍

@enekomartinmartinez
Copy link
Collaborator

@JamesPHoughton have you been able to change the configuration in RTD? And what do you think about moving the release status?

@enekomartinmartinez
Copy link
Collaborator

If we understand by being feature complete supporting all the Vensim and Xmile functions …

Fair enough, I didn't think about that! But maybe that raises an interesting point: the alpha/beta/… status may not be that informative overall, because it's unclear what it relates to for PySD. In my perception, PySD's public interface looks rather stable and it is feature complete regarding many intended use cases. For me that would already warrant a "release" or "production" status, even if there are still some further features and translation functions to be implemented.

At the same time, I see your point: It should be clear that not all Vensim and Xmile functions are available yet. However, would that not be better communicated by explicitly having something like a comparison table that shows which functions are available and which still need to be implemented? An alpha or beta status does not really convey that, in my opinion.

Either way: I think you have to feel happy with the release status and I can understand if you'd want to keep it as alpha or beta +1

Thanks for this clear explanation, maybe moving to beta could be a good approach.

@JamesPHoughton
Copy link
Collaborator

Hey @enekomartinmartinez - I was able to manually set up some active versions in the RTD, but didn't add every release, as there are so many. Here's what's currently being shown:
image
Do you think that's a reasonable set?

@JamesPHoughton
Copy link
Collaborator

Fair enough, I didn't think about that! But maybe that raises an interesting point: the alpha/beta/… status may not be that informative overall, because it's unclear what it relates to for PySD. In my perception, PySD's public interface looks rather stable and it is feature complete regarding many intended use cases. For me that would already warrant a "release" or "production" status, even if there are still some further features and translation functions to be implemented.

At the same time, I see your point: It should be clear that not all Vensim and Xmile functions are available yet. However, would that not be better communicated by explicitly having something like a comparison table that shows which functions are available and which still need to be implemented? An alpha or beta status does not really convey that, in my opinion.

I don't personally think we need to tie our alpha/beta status to feature completeness - the feature set of Vensim is notoriously bloated, and I don't think we can or even should aim to support all of Vensim's functionality. From my perspective, the alpha/beta/production status signals to the user more about how ready the software is for the average user from a reliability standpoint, and how involved we expect them to be in the bugfinding process! I'm more than happy to move to 'beta' at this point to signal the maturity that @enekomartinmartinez has achieved in the last 3(!!!) major releases. =)

@enekomartinmartinez
Copy link
Collaborator

Hey @enekomartinmartinez - I was able to manually set up some active versions in the RTD, but didn't add every release, as there are so many. Here's what's currently being shown: image Do you think that's a reasonable set?

Thanks a lot! That looks fine to me. I would show all the releases of the current major release 3.x.x, and the notorious previous major releases. Maybe it would be nice to also include the last 2.x.x release. What do you think?

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

No branches or pull requests

4 participants