These are chat archives for astropy/astropy

19th
Apr 2016
Michael Seifert
@MSeifert04
Apr 19 2016 18:41
How do I activate the appveyor python 3.5 test for one PR? I'm currently testing a workaround for the "known failures" and I want to submit a PR that (maybe) solves them. But I wouldn't like activating the appveyor test again be part of that PR.
Kumar Ayush
@cheekujodhpur
Apr 19 2016 18:53
How many slots did astropy get for GSoC?
Erik Tollerud
@eteq
Apr 19 2016 19:13
Sorry @cheekujodhpur - Google says we can't discuss those sorts of details until their official announcement, Apr 22
@MSeifert04 - I’m not sure I completely understand what you’re asking - you mean you want to trigger only the 3.5 test? If so, the easiest thing is to push up a commit that removes all the other versions in the “matrix” section of appveyor.yml . Then if it works, un-do that commit (with a [ci skip] if you don’t want to re-run the tests)
Michael Seifert
@MSeifert04
Apr 19 2016 19:16
@eteq Sorry I just tried it a few minutes ago (see astropy/astropy#4792). Maybe I should have deactivated all other tests, never actually thought of that. Sorry :(
Erik Tollerud
@eteq
Apr 19 2016 19:40
ah, no worries - that’s just fine too
they don’t take that long to run, so that would really only be worthwhile if you think you need to do a bunch of them in a row or something
Michael Seifert
@MSeifert04
Apr 19 2016 19:44
except that it did fail the test because some command cannot be run. Is there some more to it to activate another appveyor build?
Erik Tollerud
@eteq
Apr 19 2016 20:05
weird - I didn’t think so… and a bunch of other recent ones ran just fine
on the speculation that it might be transient, I tried re-running it
Michael Seifert
@MSeifert04
Apr 19 2016 20:07
oh, I just submitted a PR removing all other tests. I should be more patient :(
Erik Tollerud
@eteq
Apr 19 2016 20:08
hah, no worries
most of the time that would have been the right call - you just happened to catch me today with all the necessary windows already open ;)
Michael Seifert
@MSeifert04
Apr 19 2016 20:13
Thank you! I just hope it passes this time, I've no idea how to fix such "Powershell" Errors. :)
Brigitta Sipocz
@bsipocz
Apr 19 2016 20:27
@MSeifert04 - The other trick you can do is to temporarily change the testing command. Since all the fails are in the fits module, you can run the test only in that module while testing for the solution (unless you have access to a windows machine and know that the PR already works).
Michael Seifert
@MSeifert04
Apr 19 2016 20:37
@bsipocz Oh, that would have been even better. I must admit that I have no idea how these test matrix stuff works and I just deleted the lines I thought were right and hoped for the best. :-)
Brigitta Sipocz
@bsipocz
Apr 19 2016 20:40
yes, what you did look good. Another trick when there is a congestion (and you don't want to wait a long time in the queue, etc), is to run the tests on your fork while experimenting. Although it works only with travis (or at least I didn't figure out how to set it up with appveyor). The additional benefit is that you have the right to cancel/restart the builds, so a bit of extra space of control.
but if there is a PR already open from the given branch, the astropy travis will be triggered anyway, so it works well only before opening the PR.
Michael Seifert
@MSeifert04
Apr 19 2016 20:48
I think the additional control is replaced by some additional overhead/effort.
And it mostly was just because of this random error. The restarted build actually passed the test.
Erik Tollerud
@eteq
Apr 19 2016 20:49
Michael Seifert
@MSeifert04
Apr 19 2016 20:49
And if it's no problem I'll let someone else clean up the Testing-matrix if nobody has objections about this PR.
Brigitta Sipocz
@bsipocz
Apr 19 2016 20:49
hurray :) that means that 3.4 can be removed and we won't be as congested as yesterday night/today.
@MSeifert04 - OK, I can do that. (Actually already have a PR that removed the 3.5 until the issues are fixed, but not change it to remove 3.4)
Erik Tollerud
@eteq
Apr 19 2016 20:50
Awesome - I will go ahead and merge #4792, then
Michael Seifert
@MSeifert04
Apr 19 2016 20:51
actually there is one open issue in the PR: I don't know which exception would be raised if there is no cdll.msvcrt.
but I'm glad it passed :)
Erik Tollerud
@eteq
Apr 19 2016 20:52
Ok, gotcha. Do you want to try to figure that out, or should I merge it now to get 3.5 working, and treat that as a separate issue?
Michael Seifert
@MSeifert04
Apr 19 2016 20:54
I actually don't know how to figure it out. I only have this one computer where it's present.
Brigitta Sipocz
@bsipocz
Apr 19 2016 20:54
astropy/astropy#4791 is the one for the matrix cleanup.
(once #4792 is in, or just before)
Michael Seifert
@MSeifert04
Apr 19 2016 21:00
I'll let you decide if there should be another issue for this or only on-demand.
Thanks for the test-matrix change :) I think that'll probably require a rebase because I've removed the allowed-failures as well.
Brigitta Sipocz
@bsipocz
Apr 19 2016 21:02
Ohh, sure. Will do it once yours is in.
Erik Tollerud
@eteq
Apr 19 2016 21:21
Ok, #4792 is in
@bsipocz - maybe rebase or otherwise push up a non- [skip ci] commit now to #4791?
Michael Seifert
@MSeifert04
Apr 19 2016 21:24
@eteq I forgot to include it into the commits/inital comment but I think the PR solves astropy/astropy#4342
Brigitta Sipocz
@bsipocz
Apr 19 2016 21:25
rebased, but with the [skip ci]
do you want it again? (now only the two lines are deleted after the rebase)
Erik Tollerud
@eteq
Apr 19 2016 21:26
Oh, you’re right, that’s a pretty easy one - no need, merged it ;)
@MSeifert04 - I’m right in my assumption that #4342 is effectively tested somewhere in the fits.io tests, right? I.e., no need for a regression test
Michael Seifert
@MSeifert04
Apr 19 2016 21:31
all the previous test failures on python 3.5 were because these warnings were catched (and the following assert len(catched_warnings) == xx triggered the test failures
Erik Tollerud
@eteq
Apr 19 2016 21:31
Assuming so, I’ll just add a changelog entry in master. I was thinking this was unique to appveyor, but I now understand it’s a more general windows issue (and hence should have been in the changelog
oh, wait, so it’s not actually that the dll is the wrong version, but rather that the warnings are being caught?
Put another way, @MSeifert04 : might a user on py 3.5.x on Windows now find that something failed before but now works? (That’s more-or-less the “does it get a changelog entry” rule)
Brigitta Sipocz
@bsipocz
Apr 19 2016 21:36
(unrelated side note, that I love that gitter shows whether issues/PRs referenced are open or closed, hopefully github will have some similar features, too)
Erik Tollerud
@eteq
Apr 19 2016 21:37
(yeah, I was just noticing that too. Fanciness! :clap: )
Michael Seifert
@MSeifert04
Apr 19 2016 21:37
@eteq Actually embray said it would only in very rare cases really affect someone. Currently it was a lot of false positive warnings.
I need to check where he had posted this
Michael Seifert
@MSeifert04
Apr 19 2016 21:40
Actually there he is saying that it needs some refactoring of _File. That would actually be bad. I haven't actually checked for file-handles directly.
but the cdll.msvcrt should have all the attributes of the previous loaddll. At least I haven't found any difference (yet).
Erik Tollerud
@eteq
Apr 19 2016 21:42
OK, but do you agree that a user might have encountered the problem that #4792 fixes?
if so, I can just add a changelog entry noting it just in case - we just won’t backport it to 1.1.x if it seems like a very corner case
Michael Seifert
@MSeifert04
Apr 19 2016 21:43
well it was just a nuisance to have these warnings.
ok, maybe to explain it this way: I really, really think (and hope) that this PR doesn't change anything except that the warning isn't issued when it's ignoreable
Erik Tollerud
@eteq
Apr 19 2016 21:46
ah, ok. Then we’ll leave it be
Erik Tollerud
@eteq
Apr 19 2016 22:46
@bsipocz - while testing a fix for #227 I noticed warnings like this comming out of build_sphinx (with sphinx 1.4 installed and py 3.x):
io/votable/references.txt:4: WARNING: py:class reference target not found: SyntaxWarning
Have you seen those before when you were testing on sphinx 1.4?