-
Notifications
You must be signed in to change notification settings - Fork 96
Improved MEX Compilation #173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
------- | ||
Namespace containing parsed arguments | ||
""" | ||
parser = ArgumentParser(prog='messenger') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 'build-messenger'
? It might be confusing that this is called 'messenger'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, should we rename the entire module to be build-messenger, or just this section of the code?
intensifies
Decided to start implementing the big merge with setuptools over current tools consolidated in the messenger module. Hooking into setuptools allows for automatic discovery of requirements and the ability to fine tune almost every aspects of the build process.
import nose | ||
args = 'nosetests -v --exe --with-cov ' | ||
if sys.version_info == (2, 7): | ||
args += '--cover-package pymatbridge' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coverage report should not be mandatory on some versions. Instead it should be an option. That is
python setup.py test --with-coverage
should achieve this effect. Is that possible to do with this?
Notice also that the coverage report on travis seems to be doing something wrong. We are getting a coverage report on some elements of IPython, instead of pymatbridge. Could you take a look at that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a .coveragerc
, see the one from scikit-image
A couple of comments:
|
So after scratching my head for an hour or so, I think I might have figured it out.
|
Okay after making these changes and running under
The complete coverage is still going down, but this is primarily due to the fact that the |
Did you push these changes to your On Fri, Apr 24, 2015 at 5:36 PM, Sang Han notifications@github.com wrote:
|
Doing it now, there's some code that I had to deactivate in this build. But let's see if I can replicate the coverage report under travis! |
So strange, for some reason now I'm getting error when running this under a python2.7 virtualenv
Have no idea where in the code is trying to read a file called Can't seem to figure this one out. I think I'm going to sleep on this one and try to tackle it again in the morning. |
Okay, now I was able to replicate the issue on Travis.
It looks like it is actually one of the build scripts running on This issue has become such a pain that I'm actually considering starting over from scratch from one of working builds in order to get things running again. |
I am not sure I entirely understand what "from one of working builds" On Tue, Apr 28, 2015 at 12:01 AM, Sang Han notifications@github.com wrote:
|
Sure, closing this PR then. |
Continuation of #172
Changes
messenger
package are included.scipy
specplatform.machine().endswith('64')
oversys.maxsize
messenger/tests