-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add travis support for basic start of spyder #2281
Conversation
Just a basic start, so far I got these versions of python running |
This is awesome @goanpeca! @ccordoba12, since you are the owner of the repo you'll have to be the one to enable it on Travis.
If you don't see the spyder repos, you'll have to go to the spyder-ide org on github and set yourself to "public" on the owner team. |
I think this PR is enough on its own, for the actual tests I prefer to open a different PR, what do you guys think? |
# | ||
|
||
elif [ "$USE_QT_API" = "PyQt4" ]; then | ||
conda create -q -n test-environment python=$PY_VERSION sphinx sip qt pyqt; |
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 should include here more packages. At least ipython-qtconsole
, matplotlib
and pandas
.
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.
Sure, any other packages besides the ones you mention?
Update
I would add the versions we are supposed to support, ? to make the test better? it would not add to much to the pipeline (for conda only!), that way we can test any particular versions that might be giving us trouble right now.
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.
No, not the minimal versions, those are usually too old. We need to test things against their latest versions.
I'd say we should test against all packages mentioned in Optional dependencies
Thanks a lot for working on this @goanpeca, besides the couple of comments I made, I think this is ready for merging :-) As you said, we can leave moving our tests to a |
Ok so I made the changes suggested, @ccordoba12, can you go to https://travis-ci.org/spyder-ide/spyder/settings and activate testing only if a .yml is present? |
@blink1073 , can you take a look at the build in my test repo? https://travis-ci.org/goanpeca/spyder/builds/56333253 I am not sure what might be the problem on the build 92.6, thanks :-) |
@goanpeca, to get PySide to work you use |
@blink1073 thanks, but why this needs to be done? doesnt spyder take care of this normally? |
That's a good question. Somewhere the |
@goanpeca, I switched on the .yml option. About mpl and PySide: we don't set the mpl backend globally, just for our Python consoles through our How much is left to merge this one? |
I have to see what generates the error with that build (it only happens on that build) and then we should be almost ready to go. |
- source activate test-environment; | ||
- QT_API=$USE_QT_API; | ||
- echo $QT_API; | ||
- python setup.py install; |
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.
This is not necessary because you're using bootstrap to start Spyder, so I think it should be removed.
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.
True, maybe we should run both from bootstrap, and then from installed... that way we can check that it is installing correctly, no?
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.
Mmmhh, but then you should need to add the --test
flag again to the app cli options.
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.
hmmmmm right... now, would it make sense to test the install or just keep the bootstrap?
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.
For now we should just use bootstrap. Testing that Spyder installs should be done separately.
The PySide error doesn't seem to do with matplotlib (mpl just issues a warning) but with conda package manager. Let me remove it to see if that fixes the problem. |
ahhh true, I never tried the conda package thinguie with pyside on python 3.... |
Ok, I removed our conda plugin, so please try again to see if everything is passing now :-) |
Ok, but I need to rebase my branch? |
I don't know how travis works exactly :-) You shouldn't in principle (because there are no conflicts between removing conda and introducing travis.yml) but you probably should so that travis tests Spyder with the new changes |
Ok, I will do this later |
Could you finish this now? I'd like to merge other PRs and (because I didn't deactivate the yml option) they appear as Failed in the GH interface. I wouldn't like to have this message for future references :-) |
There is nothing that can be done with those messages, as they where generated where there was no actual yml to test, if you want to get those messages out, people would need to rebase after this PR is merged, so that their branches include the yml etc.... And sorry cannot do this now. I can in a couple of hours. |
Ok, no biggie then :-) But I really like to merge this one ASAP! |
@goanpeca, you'll have to rebase to pick up those changes, because your branch still includes the conda package. |
Ok rebased... but now I get this error on build 8.2 from PySide.QtGui import * # analysis:ignore
ImportError: /home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/PySide/QtGui.so: undefined symbol: _ZN8QPainter10drawPixmapERK6QRectFRK7QPixmapS2_
I found this when looking at google https://groups.google.com/a/continuum.io/forum/#!topic/anaconda/sUWNlhLykAU And since anaconda recently updated to 2.2 (miniconda maybe did an update?) this maybe has to do something with that. Comments @ccordoba12 , @blink1073 ? |
That one is my fault ;-) I updated Qt on Anaconda, so the PySide packages need to be rebuilt against that new Qt version (we missed this fact, so thanks for letting us know about it :-) I'll try to do that ASAP, but for now please disable the PySide/conda combination (until I let you know that is working again). |
@Nodd, PySide is available in Conda for Python 2, but not Python 3. |
Why wont continuum make a conda package for pyside and Python3? |
Well, threads like this one show that pyside development has gone very slow: http://lists.qt-project.org/pipermail/pyside/2015-February/002251.html |
@Nodd Yes I have seen the thread, but if you are going to offer PySide for Python2 why not for Python3 ? Why not just say that "support of pyside is over" and take all of it out of continuum repos, instead of offering half of it... if there are wheels that work, why not conda packages? |
@goanpeca, I can answer that because I'm in charge of Qt/PyQt at Continuum:
The end of the history is this: Continuum is not committed to support PySide anymore because PyQt4 are more stable and better maintained Qt bindings than PySide. I think the PySide package will continue to remain in the repos because of historical reasons and nothing more. So instead of keep complaining about it, please remove the Conda/PySide combo from the testing matrix. We can still test PySide using Ubuntu and those wheels, I can't see any really problem with that. Even if we found problems with conda and PySide, no one at Continuum (meaning me and the Anaconda team) would care about it. They would just say: "please use PyQt4 instead". |
@ccordoba12 I do not have a problem in removing it from the matrix. I use anaconda and I use pyside by default, so it is very annoying that after doing
Spyder goes @#%#$^$%^#%... so yes I should be able to complain "So instead of keep complaining about it, " is unnecessary It would be better if they (continuum) did not offer pyside support at all instead of a half baked solution. |
I'll talk to them but I wouldn't hold my breath. If you are using Anaconda, you should be using PyQt4, that's all :-) Could you comment PySide from .travis.yml so we can merge this one? Then you can add it to in another PR when you make it work again. Edit: If you want to use PySide, please use the Ubuntu packages instead (that's what I do :-), and change your interpreter to point to the Anaconda one. |
@ccordoba12 give me a sec, I am doing the travis runs on a separate branch just to check that it runs before making the PR.... |
Ok, I guess now it should be fine.... |
@ccordoba12 since you are the only one doing direct commits to the repo, it would be better if you used |
For that to work I should start to do PRs for everything I do, instead of committing directly to the repo. How else would we guarantee that Spyder is passing the tests all the time if not? ;-) |
export SOURCE=`source activate test-environment` | ||
install_conda; | ||
else | ||
export TEST_PACKAGES="IPython jedi matplotlib pandas pep8 psutil pyflakes pygments pylint sphinx sympy" |
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.
After seeing the travis build logs, I think this is causing matplotlib (and all the other packages in this line) to be installed with pip
instead of using apt-get
. Two questions:
- Why not use
apt-get
, at least for mpl? The other packages are just pure Python packages, so there is no problem with usingpip
. - If that's not possible, can't use a wheel for mpl?
I say it because the workers are taking a huge amount of time to compile mpl, which is reasonable because mpl is a big package. But we shouldn't build mpl if we can avoid it :-)
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.
- Travis uses environments (with virtual env), so using apt-get will install packages for the global python which is 2.7 and that makes no sense for python 3.3, and 3.4. Travis VMs are all the same, what changes is the virtual envs....
- Yes, we can use wheels (if they are available...) for all the other packages that take too long too compile. Otherwise we would need to make our own wheels for the different packages we are testing.
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.
Then (as I said), please use a wheel for mpl. The other packages are fine.
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.
For the moment only conda then.... I will add the wheels later on...
I think we should reduce the testing matrix, and just use conda for now. And no, it's not because I'm working at Continuum :-), it's because conda packages are (almost always) up to date and quite reliable. I also say it because the wheels in http://travis-wheels.scikit-image.org/ seem outdated (there is no mpl 1.4.3 for example). At the end, the tests using conda are already passing, and do so only taking 1 min or so. So why not start with that and leave the other options for later? |
ok |
So for the moment only conda... when we have a set of wheels ready and updated we can activate the rest |
Yes, that's ok. One last question: what do you recommend for my particular workflow? Should I start making PRs instead of pushing directly to master? |
I agree with using all conda all the time, and letting you do whatever is easiest for you @ccordoba12. For instance, minrk pushes directly to master on IPython quite often. |
Lets just keep doing things as usual and see how it goes 😄. Is this ready for merging, or is there something I need to fix before? I am starting with the tests now, specifically the source code editor. Cheers |
Ok, merging then! Thanks a lot for your work on this @goanpeca, this is immensely useful! About the other tests, please start simple. Let's start with the Editor in one PR, and then the other plugins in further PRs. |
Add travis support to test that Spyder starts correctly.
Sorry I am late in the battle. I have the same issue when testing qtawesome with PySide: undefined symbol when trying to import it. Although I still think that supporting PySide is valuable, especially license-wise. Should I drop it from the conda tests? |
@SylvainCorlay, no battle here ;-), sure disabled (comment the lines...) I will add pyside from normal installs (using wheels) later Cheers |
@goanpeca it is just an idiom. / I hope anaconda will support pyside again in the future. |
The official answer is that Continuum doesn't support PySide anymore. However, I'll try to update it on my free time. |
Related to #2268
Todo: