xzos / PyZDDE

Zemax/ OpticStudio Extension using Python
MIT License
159 stars 67 forks source link

error with zSetSolve #13

Closed userPyZDDE closed 10 years ago

userPyZDDE commented 10 years ago

Hello, I start to use the pyzdde library few days ago and I would like to thank you because it's a great job. But I have an issue with the zSetSolve function. When I use it, it raise the following error : " if data: UnboundLocalError: local variable 'data' referenced before assignment " below, I put the way how I call the function : dataP = [2,0,0.0,-1.0,0] #(solvetype,pickupSurfaceNb,offSet,Scalefactor,column) dataP[1] = int(surfnum) status2 = links.zSetSolve(surfnum + 2,8,tuple(dataP))

Best regards

ulip commented 10 years ago

Hello,

the error is caused because you pass a tuple/list as solveData and zSetSolve doesn't seem to like that. For the moment you can get your code running by passing unpacked numbers like this:

status2 = link0.zSetSolve(surfnum + 2,8,2,0,0.0,-1.0,0)

Best regards Uwe

indranilsinharoy commented 10 years ago

Hello @userPyZDDE,

Thanks for your kind words. I really appreciate for reporting this issue. Looking at the code now, it is clear to be that there is a discrepancy between the function definition and the doctest! As Uwe correctly suggested, passing an unpacked sequence of values will work (for now), i.e. you can pass dataP as follows:

dataP = [2,0,0.0,-1.0,0] #(solvetype,pickupSurfaceNb,offSet,Scalefactor,column)
dataP[1] = int(surfnum)
status2 = links.zSetSolve(surfnum + 2, 8, *dataP)

However, I believe that the code needs to be fixed.

Hello @ulip ,

Thank you very much, Uwe, for the suggestion.

Now, I have a question for both of you.

One way to fix this issue is to redefine the function as def zSetSolve(self, surfaceNumber, code, solveData) instead of the current def zSetSolve(self, surfaceNumber, code, *solveData). In that case solveData is expected to be tuple/list. However, it will break any code currently using zSetSolve() passing a sequence of arguments or splatting. (I think most people may like passing the soveData as a tuple instead of unpacking it).

The other way is to leave the function definition as it is currently (and change the docstring to suggest the same). Then the solveData is a sequence of arguments.

I will really appreciate your suggestions.

Thank you very much and best regards Indranil.

userPyZDDE commented 10 years ago

Hello @ulip and @indranilsinharoy,

Thank you for your answers. The issue has been solved.

Indranil, To answer your question, personnaly I think the tuple/list solution is the best. It's more user friendly and simplier for a python beginner which wants to do Zemax calculations.

best regards clement

ulip commented 10 years ago

I would leave it as it is and maybe make it clear in the doctext that arguments are expected in unpacked form. Passing a tuple and putting a star in front of it to make python unpack it is not that inconvenient at all. As far as I know all functions in NumPy, etc. with variable argument lists work the same way. Suggestion: Wontfix and update doctext. Regards Uwe

indranilsinharoy commented 10 years ago

Thank you very much @userPyZDDE and @ulip for your inputs.

Although in the morning I was in favor of using list / tuple, by afternoon (after some deliberation) I too was thinking very much along what Uwe has suggested. That way other people's code will not break down all of a sudden; also, as Uwe has suggested, putting a star in front of a tuple/list to splatter the data is really not that inconvenient.

Tonight I will update the doctext. Additionally, I will also provide two specific examples within the doctext for the two methods of usage. I will also close the issue.

Thank you very much gentlemen, Regards, Indranil.

indranilsinharoy commented 10 years ago

Fixed.