Conversation
|
On first try! Booya! |
|
Pretty awesome job @jjangsangy! I'll let @isbadawi chime in since he's a fellow builder. |
messenger/make.py
Outdated
There was a problem hiding this comment.
Should be cfg.read(config). Can't build Octave because of this.
There are two other places where you read config.ini. You should probably reuse this function everywhere.
There was a problem hiding this comment.
Also this used to return a dict. The octave path uses % get_config() and this won't work with a ConfigParser instance.
|
Lots of little nits, but OK overall. I do like the one config file better. |
|
Ya, thanks a lot for going through and finding all those mistakes. Always nice to get code reviewed! Went ahead and refactored out all the details.
|
|
Crap! forgot a comma! |
There was a problem hiding this comment.
This will reintroduce a problem I had running pymatbridge with 32 bit python on 64 bit Windows.
If you are running 32 bit python on 64 bit windows sys.maxsize > 2**32 gives False while platform.machine().endswith('64') returns True.
There was a problem hiding this comment.
@mmagnuski Thanks for bringing that up!
Planning on dealing with that soon. The issue is that we actually have 3 different architectures.
- Operating System Architecture
- Python Architecture
- Matlab Architecture
I just built get_matlab_env() which will return the MATLAB arch, so now all we have to do is figure out which combinations of these 3 are okay to run under.
There was a problem hiding this comment.
IMO, the only one that matters for the mex file is the Matlab Architecture, which must in turn match the ZMQ Architecture.
There was a problem hiding this comment.
@blink1073 But using a 64-bit compiled MEX running on a 32-bit Python interpreter is bound to be bad right? I'm not too familiar with the build process for windows so I'll have to go look into this one.
ZMQ Architecture? Or do you mean the version of the ZMQ installation?
There was a problem hiding this comment.
My point is that we start Matlab in a subprocess and talk to it over sockets, so it doesn't matter if the architectures match.
There are 64bit and 32bit downloads for ZMQ on Windows, for example.
There was a problem hiding this comment.
Ahh I see, that makes sense.
Features
get_matlab_bin()function.messengeris a fully fledged helper module forpymatbridgemessengermodule is also a callable application, and is a direct replacement formake.py.etc...
DRYlocal.cfgfiles are deprecated. Now only single authoritativeconfig.inifile in root directory.get_matlab_env()for inspecting underlying installed matlab architecture.fetch_zmq()to downloadlibzmqsource codeStill not Implemented
libzmqheaders and library usingDYLD_LIBRARY_PATHDYLD_FRAMEWORK_PATH.