Initializing ZOSPy with fewer lines

[andibarg]

Hi all,

At the moment as many as four lines (apart from import) are needed to initialize ZOSPy:

import zospy as zp

zos = zp.ZOS()
zos.wakeup()
zos.connect_as_extension()
oss = zos.get_primary_system()

I was wondering if it has to be this way. Could zos.wakeup() not be called automatically when creating the instance via zos = zp.ZOS()? I think everyone wants to do that anyways, right? Same goes for zos.connect_as_extension() and zos.create_new_application() - why can they not return the primary system oss right away?

If there is an opportunity to turn four lines of code into two, I would take it :). What do you think?

[LucVV]

Hi @andibarg,

Thank you for adressing this topic. I think improvements can indeed be made on this side. To reflect on your points:

1. Embedding zos.wakeup() in the __init__() of zp.ZOS()

When starting with ZOSPy, I expected greater functionality for the ZOS class. Potentially also some functionality that did not require active connection to OpticStudio. Hence, I wanted to be able to initiate a zp.ZOS() instance without the necessity of the ZOS-API being available. This functionality is however not there and I think it would indeed be an improvement to embed zos.wakeup() in zp.ZOS.__init__().

2. Returning the primary system from zos.connect_as_extension() and zos.create_new_application()

I did not implement that on purpose, especially since the stand-alone mode from OpticStudio that is initiated with zos.create_new_application() can hold multiple optical systems. With this implementation, the user has to indicate which system to work with and there would be no confusion on that part. On the other hand, only one system can exist in extension mode, so it makes sense that zos.connect_as_extension() would return the primary system. @crnh suggested that we could add a keyword argument to zos.create_new_application() that specifies whether a system is directly returned. I think that would be a good solution. Curious what you think about these solutions!

[andibarg]

Hi @LucVV,

Yes, that sounds great.
I was not aware that one can get multiple optical systems. What can this be used for? To open several Zemax files simultaneously in one Python script?

It seems that for zos.create_new_application() users would always get at least one oss, so it might as well be returned directly. To get another one, you can always call oss2 = zos.create_new_system() afterwards. Of course, the suggestion by @crnh to include an argument is very reasonable and perhaps even cleaner.

[crnh]

Yes, you can indeed use it to open multiple Zemax files / optical systems in a single script. The ZOS.create_new_system method has been added recently (in version 1.0.0).

If we allow to get the optical system back immediately when connecting to OpticStudio, we should implement it in the same way in both connect_as_extension and create_new_application. It is hard to imagine cases in which the user does not want to use the primary system, so returning it by default makes sense to me.

However, we need to think about handling connection failures. Currently, the connection methods return a boolean, indicating if a connection was created successfully. If the optical system is returned instead, these methods need to handle the case in which no connection could be created, probably by raising an exception.

In any case, these changes will most likely introduce breaking changes into a very fundamental part of ZOSPy. It will probably take quite some time before we publish a release with these changes.

[andibarg]

Hi again,

I implemented some the changes to this and uploaded pull request #26. Indeed, I can now initialize ZOSpy with just two lines:

import zospy as zp

zos = zp.ZOS()
oss = zos.create_new_application()

With the few tests I have done so far, I could not see anything breaking. It is even backward compatible, meaning that the four lines mentioned above still work to initialize ZOSpy (without another assert in the main script to check connection). Note that I included asserts for the connections in connect_as_extension and create_new_application before returning get_primary_system.

I have not run a unit test, since I am currently trying to wrap my head around that :).

Let me know what you think.

Edit: Corrected example code

[crnh]

This has been implemented in a backwards-compatible manner in #26. Initializing ZOSPy can now be done with

import zospy as zp

zos = zp.ZOS()
oss = zos.create_new_application(return_primary_system=True)

In the next major version, this will become the only way to initialize ZOSPy.

[crnh]

Looking back, I am afraid we didn't put enough thought into this and focused too much on the current situation.
Apart from the more complicated unit tests, a problem with the current implementation is that it is impossible to anticipate for the breaking change in the next major version (when the return_primary_system will be removed).

A cleaner solution would be to create a new connect(mode) method, with mode either extension or standalone (the full signature would become something like connect(mode: Literal["extension", "standalone"] = "extension") -> OpticStudioSystem).
This implementation would have allowed the users to already change their code to the new initialization pattern, without the intermediate step of having to remove the return_primary_system keyword argument when we release ZOSPy 2.0.0.
(It would also make the weirdly named connect_as_standalone method redundant; imo "connect as standalone" is grammatically incorrect, it should be either "connect in standalone mode" or "connect as standalone application).

The problem is: we did already release these changes. Nevertheless, implementing it this way still results in a smoother transition to the simplified initialization of ZOSPy. It may also make the API a bit cleaner, by merging three connection methods (which differ by only a single line) into one. I would like to hear your opinions about this. Do the possible advantages of this implementation outweigh the disadvantages of introducing another (for now non-breaking) change in the API?

[LucVV]

It is a difficult choice indeed. The question is if we will have more breaking changes (thus far not if I am correct). This is a breaking change in quite a fundamental part of the package indeed.
My initial idea was to keep return_primary_system but change the default value in the next release. That way, it would not be a breaking change for people that connect with

zos = zp.ZOS()
oss = zos.connect_as_extension(return_primary_system=True)

But it will be breaking for others.

I like the idea of a single connection function in which one can specify how to connect. I think we can implement that and leave the current connection functions as is (with the 'return_primary_system' parameter). That way, we do not revert changes that people might currently use (even though we did not do an official release with them yet).

I would propose to implement the connection function in the next release, and I think it could be version 1.2 as it is not a breaking change. We can then add a deprecation warning to the current connection functions and remove them in a later version (maybe even 2.0 to give people time to switch)

[crnh]

I expect more breaking changes when we start working on more robust analysis parsers, but those will be in a different part of ZOSPy.
I agree with keeping ZOS.connect_as_extension etc. as is until the next major release. However, we have seen that having those functions return either a boolean or an OpticStudioSystem decreases the maintainability of ZOSPy and makes the unit tests more complex. I therefore agree with your proposal to remove connect_as_extension, create_new_application and connect_as_standalone in version 2.0.

[LucVV]

I think that we should aim to implement the connect function quickly and add a deprecation warning to connect_as_extension, create_new_application and connect_as_standalone.

I am thinking about what we should pick as a default for connect. While I am personally a fan of connecting as extension, creating a standalone application is more robust as default, as it does not require the user to manually do things in OpticStudio.

[crnh]

This has been implemented in #62 and just released with ZOSPy 1.2.0!
We introduced a unified connect method that replaces the methods discussed above. In the next major version, the old methods will be removed.
@andibarg Thanks for suggesting this improvement!