Skip to content

Conversation

blink1073
Copy link
Collaborator

Prior to this change, trying to run m.version() without calling m.start() gave this unhelpful message:

/home/silvester/workspace/python-matlab-bridge/pymatbridge/pymatbridge.py in _response(self, **kwargs)
    221     def _response(self, **kwargs):
    222         req = json.dumps(kwargs, cls=PymatEncoder)
--> 223         self.socket.send_string(req)
    224         resp = self.socket.recv_string()
    225         return resp

AttributeError: 'NoneType' object has no attribute 'send_string'

Also allows you to use m = Matlab().start()

@arokem
Copy link
Owner

arokem commented Feb 22, 2015

I wonder whether that's always the behavior you want. What if you intentionally stopped the Matlab process? One option is to raise an informative error instead of this one, and force users to call start themselves. Another is to raise a warning of some kind, to alert the user to the fact that this going to start matlab for them. What do you think?

@blink1073
Copy link
Collaborator Author

I think the warning is a good compromise, what do you think @isbadawi?

@isbadawi
Copy link
Collaborator

I'm not sure I see the value in auto-starting it -- explicit is better than implicit, etc. I'd prefer to just see an exception. But I don't really feel strongly either way.

Maybe just play with the API to make this less of a pain point? You could add a function that returns an already started session, since you often create and start a session at the same time anyway, or make start() return the session again instead of a boolean, so you could write pymatbridge.Matlab().start() in one step.

(Reminds me of a similar problem with Stopwatch in Guava: https://plus.google.com/+googleguava/posts/8XeVCd7iKxo).

arokem added a commit that referenced this pull request Feb 26, 2015
Start the session if you try and run code
@arokem arokem merged commit feeeb42 into arokem:master Feb 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants