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: Minor suggested documentation improvements #17

Closed
emilydolson opened this issue Jan 14, 2022 · 4 comments
Closed

JOSS Review: Minor suggested documentation improvements #17

emilydolson opened this issue Jan 14, 2022 · 4 comments

Comments

@emilydolson
Copy link

Lastly, I've got a couple suggested additions to the documentation. These aren't necessary in my mind, but I think they would be helpful and increase adoption of this framework (they certainly would have been helpful to me as I was testing it out 😄):

  • The blank project template is super helpful. I'd add an explicit link to it somewhere in the documentation so it's easier to find (or if it's already there, somewhere more prominent).
  • Some of the comments in empty_project.html are currently wrong (e.g. // Set half (50%) of the Gridmodel's grid points to 1 (alive)), which makes it harder to figure out how it's working. I'd recommend fixing them.
  • This is a bigger project, but more extensive documentation of what all the functions available to the user are would be very helpful. For example, this would include a list of all the methods of the random number generator.
  • More specific documentation about what exactly functions are expecting as arguments and providing as returned values. It took me a while to figure out what the getNeighbor methods return, or what the third argument to setGridpoint should look like.

This issue is part of my JOSS Review (openjournals/joss-reviews#3948)

@bramvandijk88
Copy link
Owner

I have fixed the empty_project.html file. I agree with you that I'm not sure it is super useful, but I decided to leave it there anyway for when users prefer to start with a nice empty file. It's not like the empty project is "in the way" or anything ;D

About the last two points: are you asking for anything beyond the already provided JSdocs? https://bramvandijk88.github.io/cacatoo/jsdocs/index.html

@emilydolson
Copy link
Author

You might have misread my comment about the empty_project.html template - I think it is helpful! I found it really useful in trying out Cacatoo. 😄. Those changes look good!

For the third point, it is entirely possible that everything I was looking for is in the JSdocs and I just failed to find it. Is there a list of random number generator methods in there? I looked for a while and couldn't find it.

For the last point, I definitely would have found it helpful to have more detail beyond what is currently there. For instance, I can see in the documentation that the third argument to setGridpoint is a Gridpoint, but because classes in Javascript are so flexible, it wasn't obvious to me what a Gridpoint object should look like. An example would have been super helpful here.

@bramvandijk88
Copy link
Owner

Actually, you're right that the external libraries used (the mersenne-twister RNG, the odex library, dygraphs) are not in the JSdocs. These were downloaded precisely in the format as they were available on other repositories, and some of them are minimised or simply were missing the docstrings to begin with. In most cases, I think that's fine, as nobody needs to know how dygraphs works under the hood (and they can look it up if necessary).

However, you are on point about the RNG functions being really useful to explain. I've added a page in the DOCs to explain the functions shipped with Mersenne-Twister.

Finally, about the contents of the "gridpoint" objects, it's actually not predefined at all. Gridpoint is pretty much an empty object, which only comes with a few functions for copying properties from template-objects. One of the reasons I decided to use javascript for this tool is to allow users to put "whatever" into their grid points. I have modified the docstrings to clarify that a typical user will use the output of getGridpoint to feed to the third argument of setGridpoint. Indeed, there's a lot of flexibility in Javascript, which makes documenting things like this a bit.... fuzzy. :/

@emilydolson
Copy link
Author

This looks perfect! Thanks!

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

2 participants