-
Notifications
You must be signed in to change notification settings - Fork 10
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
Naming, folder structure, and general documentation #1
Comments
Thanks for pointing out! We did all the suggested changes. Thanks a lot! |
Thanks. I'll take a deeper look at this in a few minutes to try and see exactly what you did. Generally speaking, it's more useful to say exactly how you addressed each suggestion, so I don't have to go searching. Hopefully Bob can give you some guidance here: even though JOSS isn't a paper journal, general principles of helping your reviewers understand your revise + resubmit still apply. I've found this article helpful, as a grad student author: https://ieeexplore.ieee.org/stamp/stamp.jsp?arnumber=5663683 If this was a traditional journal submission, the onus would be on the authors to describe the changes, not the reviewers to discover them. Otherwise, this wouldn't pass the R+R stage. |
^^ the above is important specifically because, as I'm going through the repository again, some of the issues raised were not addressed. I'll put them here as I see them.
|
Hi @apsabelhaus. Sorry for the inconvenience. |
@apsabelhaus, are you aware of any MATLAB-based open source toolbox that does installation differently? In stedy as well as in a couple of other MATLAB-based softwares (m_mhw, UniNE-CHYN) accepted in JOSS, I find the same approach of 'add path' being used during installation, and 'Set Path' in case you don't want to run the setup file every time MATLAB is restarted. |
@vaishnavtv I'm not aware of anything that makes this easier, unfortunately. I use the same procedure as you do with STEDY. An easy way to fix this is for the documentation to be clear that setup.m must be run every time MATLAB opens, before any other file. |
@apsabelhaus Thanks for letting us know! We added the following sentence to our software front page README.md. |
In response to the points from the word doc - Name: the new name is much more helpful! It may be good to include your responses from the word doc as part of the technical manual or something like that, so others understand MOTE's purpose succinctly. Folder structure: everything is much more logical now, thanks for this effort. Setup.m: your sentence in README does the job! I'll close this issue, the repository meets JOSS' requirements much better now! |
I would recommend that the authors take a second look at some of the naming in the repository. As an example, the name:
Tensegrity Engineering Analysis Master
is a nice acronym, but is relatively vague as to its purpose. Specifically, "engineering analysis" is very broad. It looks like this library does statics and dynamics, but there's lots of other analysis for tensegrity structures that might be considered. A more specific name related to exactly the library's purpose would be helpful: example, how does this software differ from STEDY (https://github.com/isrlab/stedy)? This would help with the "purpose" requirement for JOSS.
Similarly, the words "main" and "master" don't have much meaning to me. "Master" as in master branch? "Main" as in "core components of the library?" The "Master" in the repo title seems a bit redundant. (Also, there's a trailing "-" in the repo name, in case that was missed, which makes it a pain to type out the directory name.)
The folder structure is hard to understand. What's the difference between "tensegrity examples" and "test examples"? Both have "statics" and "dynamics" subfolders. Both are tests, correct? And there seem to be tensegrity structures in both folders.
Your 'setup.m' file also doesn't seem to work as intended. MATLAB's "addpath" is only valid for one session, and needs to be run every time you re-open MATLAB, so it's not "run only once" if that's what you mean. (P.S., to fix this, please don't use 'savepath' to permanently change the saved path on a user's computer, it takes a long time to untangle unwanted behavior that way.)
One approach (you don't have to do this, there are other ways) is to have the path additions at the start of any particular script so the path gets re-added each time and each script is independent. That's what we do in tiso for example (https://github.com/apsabelhaus/tiso/blob/master/is_examples/2d/horizSpineMultiVert_2d.m).
There are duplicates of files in various places with the same name. For example, in both Main/Tensegrity_Statics_Main and Test_Examples/Statics there are D_Bar_Statics.* files and Main_equilibrium_D_Bar.m, among others. Are they different? If not, the authors will need to eliminate code duplication as much as possible before publication (minimize the copy-paste.)
Before any further review, I'd ask that the authors take another look at all this organization, eliminate duplicate files and duplicate or redundant or overlapping folder structures.
It might also be good to proactively look over how we reviewed STEDY (openjournals/joss-reviews#1042) and some of the issues that were closed as part of their review (https://github.com/isrlab/stedy/issues?q=is%3Aissue+is%3Aclosed).
In particular I'd point the authors to the thoughts on using MATLAB's 'help' command for documentation, which is one way to meet JOSS' documentation requirements for the core API. (isrlab/stedy#8).
It's wonderful that you have a documentation PDF, but answering some of these questions in it - what does some piece of code do? What results are expected? - would be helpful.
The text was updated successfully, but these errors were encountered: