spitschan / SilentSubstitutionToolbox

Toolbox to simulate colorimetric observers for evaluation of photoreceptor isolation
Other
7 stars 5 forks source link

SSTReceptorDemo broken #7

Closed DavidBrainard closed 7 years ago

DavidBrainard commented 7 years ago

This is because a path on Manuel's machine is hard coded into the demo. Could use tbLocate to find the directory of the toolbox, but you are then requiring TbTb to use SST, which does not seem optimal for most of the world. You can also use fileparts to get it.

Note that there is a call to tbUse at the start of this demo, again probably not wanted. Or at least, you could first do a check that SST is on the path, and only try tbUse if it is not there. For that, probably there are many ways, but a call to which with some function that is in SST but not in the current pwd might work.

spitschan commented 7 years ago

TbTb is necessary because of SST's dependencies on the BrainardLabToolbox and PTB-3. Path issues are resolved as of https://github.com/spitschan/SilentSubstitutionToolbox/commit/6409d62e39dbc7b2863e6f496c052b9ee2852f6d

If there is a non-TbTb way to get those dependencies, please do let me know. I don't see one.

It would be great if you could run the demo and report back if it works as a first-order goal. Second-order goal is documentation, which is tracked in https://github.com/spitschan/SilentSubstitutionToolbox/issues/8

DavidBrainard commented 7 years ago

Yes, this now runs for me. Didn't examine what it did, but it did not crash.

With respect to TbTb. Presumably there are install instructions for SST (I didn't look), in the README or gitHub Wiki. It seems fair to assume that whoever is running the demo has properly installed the toolbox. So I would not require that they have TbTb. Rather, the install instructions should say what they need to have on their path and where they can get it.

The demo can then check for proper install (as indicated above, but check for PTB and BLTB as well as SST) and complain if is not there. Maybe encapsulate as CheckSSTInstalled function. If stuff is not installed, a message could come out pointing to the install instructions and then an error get thrown.

This is all on the view that we intend SST as a tool that will be helpful outside of our lab. We headed that way when we created silentsubstitutiontoolbox.org and used the toolbox as a resource in the natural illumination paper.

As great as TbTb is, I think there are likely to be people who could make good use of SST but who will not want it.

spitschan commented 7 years ago

Closing this issue and opening a new one to address dependencies