search

ISET / iset3d-v3

LEGACY: Replaced by iset3d (v4) -- Read a PBRT file. Edit the parameters. Render an ISET scene or optical image.
MIT License
36 stars 8 forks source link

contains() doesn't exist in older versions of Matlab #17

Closed DavidBrainard closed 5 years ago

DavidBrainard commented 5 years ago

iset3d has many calls to the function contains(). This does not exist prior to 2016b, and in particular not for the 2015x version of Matlab I have set up on the Jenkins server.

A cursory check suggests that the calls to contains are not doing anything fancy, just checking whether the second string argument is found within the first.

If this is right, I could write a TF = piContains(a,b) that worked the same way but that was based on strffind. This would be easy as long as you're not using the ability to pass multiple options in b and return the TF status for each (and maybe not hard in that case either.) And as long as you are not using the ignore case key/value pair that contains() has and strfind() does not.

I guess there could also be speed differences, so if you think calls to contains() are likely to be time consuming relative to other things, that would be another reason to hesitate.

Let me know what you think.

tlian7 commented 5 years ago

Hi @DavidBrainard,

I ran into this problem as well when trying to get ISET3d up and running on student laptops, some of which had older versions of MATLAB. I asked them to replace instances of contains() with strfind(), which is similar to what you brought up. Everything seemed to work with strfind(), so I don't forsee any problems with replacing contains() as you suggest.

I doubt the speed issue will make a big difference for the calculations we're making.

DavidBrainard commented 5 years ago

I made this change globally in iset3d. I would have done it on a branch and issue a PR, but I don't know how to get Jenkins to validate a branch so I merged into the master.

Before I merged, I created a branch called beforefixcontains that has the version before I did the global find/replace. If something is messed up, we can restore that as the master.

But am going to be optimistic and close this issue.

Jenkins validations still don't run, but it is now a different problem.