These are chat archives for astropy/astropy

13th
Nov 2015
Thomas Robitaille
@astrofrog
Nov 13 2015 10:03
@bsipocz - I'm investigating the failure on AppVeyor now, will let you know if I find anything
Thomas Robitaille
@astrofrog
Nov 13 2015 11:58
@bsipocz - I'm onto someting
If you look at this build here:
it looks like even after activate test is run, python is still set to 2.7
Brigitta Sipocz
@bsipocz
Nov 13 2015 11:59
ouch
and why didn't it happened before?
Thomas Robitaille
@astrofrog
Nov 13 2015 11:59
must be something to do with powershell
it wasn't in the powershell before
I don't yet know how we get around this
but investigating
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:00
should we download miniconda3 instead of miniconda?
Thomas Robitaille
@astrofrog
Nov 13 2015 12:00
No
I think we should set the path manually to point to the env
will push a commit to ci-helpers to test
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:01
:+1:
Thomas Robitaille
@astrofrog
Nov 13 2015 12:01
For the 2.6 build it's the same
it's still using 2.7
basically I think that running activate isn't having an impact on the path in the powershell
Thomas Robitaille
@astrofrog
Nov 13 2015 12:06
Just did this
astropy/ci-helpers@7674eb4
Not sure if += means append at start or at end
we'll see I guess!
Thomas Robitaille
@astrofrog
Nov 13 2015 12:14
At the moment I'm just pushing a few debugging commits to ci-helpers but of course in future we'll have to be a lot more careful
Thomas Robitaille
@astrofrog
Nov 13 2015 12:22
Ok, there were several issues
(all subtle)
in ci-helpers, when we added the conda directory to the path, we added it to the end, so that the conda command worked fine, but since there is a system installation of python, when we did python --version it was picking up the system one
the next issue is that in the main appveyor script we only add the main environemnt to the path, not the test environment
so I think the key is to add activate test back to appveryor.yml (in addition to having it in ci-helpers)
I'm testing to see if that works
also I think there is still a problem that activate test doesn't work in the powershell
many issues in one go!
:-/
will summarize the changes to make once I figure it out
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:27
and wasn't that the way it was run before, too? anyway really a big thanks for fixing this, to see the way appveyor runs is still a bit messy for me
Thomas Robitaille
@astrofrog
Nov 13 2015 12:32
Well the thing is that before a lot of these things didn't happen inside powershell
activate test was run in the main appveyor file
and that worked
but somehow here running activate test inside the powershell (1) doesn't seem to even work in the powershell and (2) doesn't impact the variables outside the powershell
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:33
ohh, ok indeed.
Thomas Robitaille
@astrofrog
Nov 13 2015 12:33
Ok, so I think the necessary changes to ci-helpers are done
The changes that need to be done for appveyor.yml are
Basically just add activate test after here:
    - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%"
    - "activate test"
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:33
first impressions of powershell was that it's a mess, now second impression is that it's a big mess ;)
Thomas Robitaille
@astrofrog
Nov 13 2015 12:33
hehe
indeed
I'll update the instructions for ci-helpers to mention that, and we'll also need to update the other PRs
that means the other ones were always testing in the root environment with Python 2.7
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:35
ok, right. Do you open a PR for the PRs or shall I make the changes?
Thomas Robitaille
@astrofrog
Nov 13 2015 12:35
Can you make the changes?
since it's just one line to add, a bit overkill to have PRs on PRs
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:36
OK :)
I also put back all the dependencies if it passes
Thomas Robitaille
@astrofrog
Nov 13 2015 12:37
you mean matplotlib?
only put the ones that were there before though
Brigitta Sipocz
@bsipocz
Nov 13 2015 12:37
and pandas
Thomas Robitaille
@astrofrog
Nov 13 2015 12:37
ok
ok, sounds good
Fingers crossed that it passes now...
Thomas Robitaille
@astrofrog
Nov 13 2015 12:43
Huh I'm confused, it's trying to install numpy from source
Ah I think it's still not installing the dependencies into the test environment
looking into it (that should be a pure ci-helpers change)
Restarted the build
Fixed it in ci-helpers I hope: astropy/ci-helpers@dd22906
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:17
Hurray :fireworks:
Thomas Robitaille
@astrofrog
Nov 13 2015 13:18
:tada:
I pushed the activate test change to package-template too
and I guess we don't test astroquery on windows (that's for another time)
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:20
ouch, I've pushed to that open PR, too
Thomas Robitaille
@astrofrog
Nov 13 2015 13:20
ah, which one?
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:20
no, it's not yet activated with astrowuery, but it's with photutils
astrofrog/package-template#12
Thomas Robitaille
@astrofrog
Nov 13 2015 13:21
Ok sounds good - thought there was something wrong when you said 'ouch' ;)
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:23
rebased
though there isn't anything important in it, only a bit of a cleanup
Thomas Robitaille
@astrofrog
Nov 13 2015 13:23
ah right sorry yes I understand now
forgot that was a PR to my PR
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:28
photutils is also updated
Thomas Robitaille
@astrofrog
Nov 13 2015 13:28
Ok, thanks!
All right, now we just need to wait for Travis and AppVeyor to catch up :clock10:
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:30
now a real ouch
looks like that the conditional in powershell is not working and thus astropy doesn't get installed
Thomas Robitaille
@astrofrog
Nov 13 2015 13:37
Ok checking
Brigitta Sipocz
@bsipocz
Nov 13 2015 13:38
i'm on it
have two hints to check out
Thomas Robitaille
@astrofrog
Nov 13 2015 13:39
ok sounds good
thanks!
Thomas Robitaille
@astrofrog
Nov 13 2015 13:47
@bsipocz - another small thing - it turns out that the split method can only be called if CONDA_DEPENDENCIES is not empty:
So we might have to add conditionals for that too
Maybe we should first install Numpy and the conda_dependencies
and then have a conditional for astropy
so something like:
if CONDA_DEPENDENCIES is "":
    conda install numpy=...
else
    conda install numpy=... CONDA_DEPENDECIES.split

if astropy is not null:
  conda install numpy=... astropy=...
pseudo code obviously ;)
Brigitta Sipocz
@bsipocz
Nov 13 2015 14:11
good catch
Brigitta Sipocz
@bsipocz
Nov 13 2015 14:55
astropy version checking works finally, but don't have any packages in mind where we could test the empty dependency list
Brigitta Sipocz
@bsipocz
Nov 13 2015 15:36
OK, shallow clone is fixed, I've put the full path there (I think it makes sense anyway as all the other packages using it should do the same)
seems like it's finished, so can I send the e-mail?
Thomas Robitaille
@astrofrog
Nov 13 2015 15:38
The package-template tests the empty dependency list
I'll trigger a new build there
@bsipocz - feel free to send the email! No need to wait on package-template
The other three (astropy, astroquery, and photutils) look good!
Thanks for fixing all this!
Actually the package template needs to include Cython so not empty
but it didn't crash when it was empty so I think it's all good
i.e. I had it empty, and it didn't crash until it got to the actual testing, so the syntax for the powershell worked
now I'll add Cython
Thomas Robitaille
@astrofrog
Nov 13 2015 15:44
(package-template needs Cython in dependencies because it actually includes Cython code)
Brigitta Sipocz
@bsipocz
Nov 13 2015 15:50
yes, sorry about that, I've cleaned out the dependency list not thinking that the Cython is needed for the template.
Thomas Robitaille
@astrofrog
Nov 13 2015 15:51
No problem!
Thomas Robitaille
@astrofrog
Nov 13 2015 16:03
@bsipocz - just realized we can't use - git clone git://github.com/astropy/ci-helpers.git on Travis, if we do then Travis won't be tested e.g. the pull request but the master version
will try and look for another fix
Brigitta Sipocz
@bsipocz
Nov 13 2015 16:03
ohh, yes of course you're right.
then there is a --depth option, maybe with that one?
Thomas Robitaille
@astrofrog
Nov 13 2015 16:04
I have a PR for that and adding more tests
astropy/ci-helpers#7
yep, that's what I'm trying
Thanks for sending the email!
and thank you for all your work on that this week!
Brigitta Sipocz
@bsipocz
Nov 13 2015 16:08
thanks for your help, you did the tricky bits :)
Thomas Robitaille
@astrofrog
Nov 13 2015 16:08
it was a team effort! :clap:
From now on we should make changes to ci-helpers via pull requests I think (once I've got the more detailed .travis.yml passing)
Now we've advertised it I mean
Brigitta Sipocz
@bsipocz
Nov 13 2015 16:12
OK, sounds reasonable.
Thomas Robitaille
@astrofrog
Nov 13 2015 16:19
I'm very confused:
> TEST_CMD='python -c "import sphinx"'
> $TEST_CMD
  File "<string>", line 1
    "import
          ^
SyntaxError: EOL while scanning string literal
(this is in bash)
any idea how to get that to work?
ah it works if I use eval
i.e. eval $TEST_CMD
weird