These are chat archives for astropy/astropy

10th
Sep 2014
Thomas Robitaille
@astrofrog
Sep 10 2014 09:50
@astrocaribe - are you around?
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:51
@astrofrog I'm here! Thanks for your help today! Now, how do I go about cleaning my giant mess?
Thomas Robitaille
@astrofrog
Sep 10 2014 09:51
first, thanks for all your work on this! git has a steep learning curve, so it takes a while to get used to
can you first give the output (just to check) of git remote -v
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:52
No worries! It's been a great learning experience!
fenrir-2:/Users/tommy/Documents/Dropbox/STScI/Astropy_Sprint/astropy > git remote -v
origin https://github.com/astrocaribe/astropy.git (fetch)
origin https://github.com/astrocaribe/astropy.git (push)
upstream https://github.com/astropy/astropy.git (fetch)
upstream https://github.com/astropy/astropy.git (push)
Thomas Robitaille
@astrofrog
Sep 10 2014 09:52
Ok, so we're going to do a quick rebase for the purpose of cleaning up the history
So to start off, do git fetch upstream
Stuart Mumford
@Cadair
Sep 10 2014 09:52
boo!
Thomas Robitaille
@astrofrog
Sep 10 2014 09:53
Once that's done, do git rebase -i upstream/master and copy and paste the view you have here (the list of commits)
just so I can make sure we see the same thing
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:53
I'm assuming everything were going to do is from my issue_2658 branch?
Thomas Robitaille
@astrofrog
Sep 10 2014 09:53
yes
sorry, should have specified that
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:54
No worries, I thought that was the case...
Thomas Robitaille
@astrofrog
Sep 10 2014 09:55
Ok, so let me know what the list of commits looks like when you do git rebase -i upstream/master - it should open up some editor (usually pico or vi) and show a list of commits and commit messages
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:55
1 pick 4482951 Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'
2 pick e3b5f02 Included a percentage descriptor on the left of the progress bar.
3 pick 946e020 Moved IPython widget/display import commands to ProgressBar.init, to be executed only if interactive mode is selected.
4 pick 41a22ba Removed test comments in ProgressBar.update_interactive().
5 pick 75807ff 1. Made update_console() and update_interactive() methods transparent. 2. Changed _pb attribute to _widget for clarity. 3. Removed redundant check for widget attribute instance. 4. Moved widget module imports to _update_interactive(). 5. Formatted ONLY percentage output of interactive progress bar.
6 pick 51b1d4d Added the handling of value=None within _update_console().
7 pick 38916df Added value=None logic in update() method; not currently functioning.
8 pick dd62a31 Added value=None logic to update method. However, new issue has popped up; no progress bar animation in either console or interactive views in the Notebook.
9 pick 812492c Several fixes to init, update, _update_console and _update_interactive methods and modules.
10 pick e9f6741 Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'
11 pick 5414615 Temp fix for missing console progressbar.
12 pick 423c681 Fixes to console.py.
13
14 # Rebase a6234a4..423c681 onto a6234a4
15 #
16 # Commands:
17 # p, pick = use commit
18 # r, reword = use commit, but edit the commit message
19 # e, edit = use commit, but stop for amending
20 # s, squash = use commit, but meld into previous commit
21 # f, fixup = like "squash", but discard this commit's log message
22 # x, exec = run command (the rest of the line) using shell
23 #
24 # These lines can be re-ordered; they are executed from top to bottom.
25 #
26 # If you remove a line here THAT COMMIT WILL BE LOST.
27 #
28 # However, if you remove everything, the rebase will be aborted.
29 #
30 # Note that empty commits are commented out
Thomas Robitaille
@astrofrog
Sep 10 2014 09:55
ok thanks, just checking
Ok, so on lines 11 and 12, replace pick by squash
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:56
Sorry, very ugly output... Basically a list of commits, I'm assuming the majority of the commit history?
Thomas Robitaille
@astrofrog
Sep 10 2014 09:56
these are all commits in your branch that are not in master
which looks good, but change lines 11 and 12 to:
11 squash 5414615 Temp fix for missing console progressbar.
12 squash 423c681 Fixes to console.py.
all the rest can stay as-is
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:57
Got it. At this point I save, yes?
Thomas Robitaille
@astrofrog
Sep 10 2014 09:57
Once you've done that, save and exit the file
now it may give an error message - if it does, can you paste it here?
otherwise, does it say 'rebase successful'?
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:58

fenrir-2:/Users/tommy/Documents/Dropbox/STScI/Astropy_Sprint/astropy > git rebase -i upstream/master

error: could not apply e9f6741... Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply e9f674152ef565b0dbcaec2b26f343a50a245d10... Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'

Thomas Robitaille
@astrofrog
Sep 10 2014 09:58
Ok, so there is a conflict, which we have to solve, open up console.py and search for <<<
do you see anything like that?
there should be a block like:
<<<
stuff here
===
stuff here
>>>
Tommy Le Blanc
@astrocaribe
Sep 10 2014 09:59
Yep, I do. This is what I saw about 1hr ago that freaked me out :)
Thomas Robitaille
@astrofrog
Sep 10 2014 10:00
ok, can you paste that block here?
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:00
There a re several, one sec...
Thomas Robitaille
@astrofrog
Sep 10 2014 10:00
Oh wait a second, I know what's happening
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:00

<<<<<<< HEAD

    if not interactive:

=======
if interactive:
self.update_interactive(0)
else:

e9f6741... Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'

Thomas Robitaille
@astrofrog
Sep 10 2014 10:00
that commit appears twice in the history
ok, let's do the following, close console.py
and in the terminal do:
git rebase --abort
let me know once that's done
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:01
Done. No output...
Thomas Robitaille
@astrofrog
Sep 10 2014 10:02
Ok, now, do git rebase -i upstream/master again, but this time, simply remove lines 10, 11, and 12
So remove these lines:
10 pick e9f6741 Modified ProgressBar() module to display output to a console or iPython notebook, depending on an input 'interactive argument.'
11 pick 5414615 Temp fix for missing console progressbar.
12 pick 423c681 Fixes to console.py.
We're reverting to how things were at 812492c basically
Then save and close and it should rebase without errors
Any luck?
Stuart Mumford
@Cadair
Sep 10 2014 10:03
@astrofrog never say that :p
Thomas Robitaille
@astrofrog
Sep 10 2014 10:03
@Cadair: shhhhh
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:03
Got it! No errors...
Stuart Mumford
@Cadair
Sep 10 2014 10:03
:D
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:03
Lol!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:04
@astrocaribe - great, can you now push to your remote branch, so git push -f origin issue_2658
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:04

fenrir-2:/Users/tommy/Documents/Dropbox/STScI/Astropy_Sprint/astropy > git status
On branch issue_2658
Your branch and 'origin/issue_2658' have diverged,
and have 32 and 15 different commits each, respectively.
(use "git pull" to merge the remote branch into yours)

nothing to commit, working directory clean

Sorry just wanted to post this to not loose... One sec...
Thomas Robitaille
@astrofrog
Sep 10 2014 10:04
Ok, sounds good
ok, now check here: https://github.com/astropy/astropy/pull/2789/commits - you can see the history is cleaner now
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:05

fenrir-2:/Users/tommy/Documents/Dropbox/STScI/Astropy_Sprint/astropy > git status
On branch issue_2658
Your branch is up-to-date with 'origin/issue_2658'.

nothing to commit, working directory clean

Thomas Robitaille
@astrofrog
Sep 10 2014 10:05
Great!
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:06
Yep, I can see that now!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:06
We're done, this should be it!
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:06
Does this submit another Travis test build?
Thomas Robitaille
@astrofrog
Sep 10 2014 10:06
We should see in 1 minute if there are any PEP8 issues
But those can wait, so get some sleep! Thank you for your patience!
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:07
Yep, it did redo the Travis tests. I'm assuming since I blew away part of the history, it needs to retest...
Thomas Robitaille
@astrofrog
Sep 10 2014 10:08
Just FYI this build tests the PEP8 compliance (only a few checks): https://travis-ci.org/astropy/astropy/jobs/34897115
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:08
Of course! Thanks again for your help! Despite the frustration of messing things up, it's definitely a learning experience!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:08
Ok, so just FYI there are three PEP8 errors:
astropy/utils/console.py:562:28: W291 trailing whitespace
astropy/utils/console.py:620:1: W293 blank line contains whitespace
astropy/utils/console.py:621:59: W291 trailing whitespace
but to fix them you just fix them and make a new commit, no messing around with rebasing
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:09
Yep! Got it. I'll fix now, and hopefully that'll take care of it!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:09
Thanks for all your work on this!
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:09
You're most welcome! It's good fun! And I've learned a ton!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:09
It'll be smoother in future, the first rebase is always a mess :)
Adam Ginsburg
@keflavich
Sep 10 2014 10:10
I can second that.
Stuart Mumford
@Cadair
Sep 10 2014 10:10
all rebases are always a mess
:-p
can be useful though
Thomas Robitaille
@astrofrog
Sep 10 2014 10:10
well it's never fun when there are conflicts
Adam Ginsburg
@keflavich
Sep 10 2014 10:10
Is everything in astropy/astropy#2793 compatible with astropy/astropy#2789 ?
Stuart Mumford
@Cadair
Sep 10 2014 10:10
@astrofrog sometimes merge commits are worth it ;)
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:11
And I bet not making the rookie mistakes is even better! :smile:
Thomas Robitaille
@astrofrog
Sep 10 2014 10:11
@keflavich - last time I checked it, astropy/astropy#2793 still didn't offer a complete solution because I don't know if there is an easy one
Adam Ginsburg
@keflavich
Sep 10 2014 10:12
@astrofrog - can you help me understand the problem? It is if you start an ipython "server" and connect different clients to it?
astropy#2789 still relies on astropy#2793 to be used, no?
Thomas Robitaille
@astrofrog
Sep 10 2014 10:12
For now we're just going to merge astropy/astropy#2789 and have it be manually opt-in
Adam Ginsburg
@keflavich
Sep 10 2014 10:12
ahhh, ok
Thomas Robitaille
@astrofrog
Sep 10 2014 10:13
Ok, to see the issue, start up a notebook then connect to the same kernel from a qt console, e.g. ipython console --existing dfb1dc79-26d1-4f0e-ae53-11052494db82
Adam Ginsburg
@keflavich
Sep 10 2014 10:13
@astrofrog - #2793 won't work in all cases, but are there any situations in which it would be disastrous?
Thomas Robitaille
@astrofrog
Sep 10 2014 10:13
both share the same kernel
Adam Ginsburg
@keflavich
Sep 10 2014 10:13
OK, I'll try that (I've never used a qt console before)
Thomas Robitaille
@astrofrog
Sep 10 2014 10:13
@keflavich - good question
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:15
@astrofrog Fixed and committed! I'll add to the CHANGES.rst later today or tomorrow, once all tests pass (or after I get some sleep)...
Thomas Robitaille
@astrofrog
Sep 10 2014 10:15
@astrocaribe - sounds good! Thanks for your dedication! :)
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:16
:thumbsup:
@astrofrog Thanks for your patience in guiding me through the rebase!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:17
@keflavich - it may very well be that there is no clean universal solution - maybe in the case where it's ambiguous we can just try and use the interactive progress bar and maybe it crashes if it can't be displayed?
Adam Ginsburg
@keflavich
Sep 10 2014 10:17
@astrofrog - I see now. It looks like it must be possible to access any kernel (notebook or otherwise?) via a console app, but it's not obvious to me that you could access a console-initialized session from a notebook
Thomas Robitaille
@astrofrog
Sep 10 2014 10:18
That's true
Adam Ginsburg
@keflavich
Sep 10 2014 10:18
@astrofrog - that would be great, but in the past, the progressbar worked in the notebook and just spewed out GBs of junk into the JSON. Does astropy#2789 prevent that from happening?
Thomas Robitaille
@astrofrog
Sep 10 2014 10:18
@keflavich - I think the progress bar currently doesn't output anything in the notebook as of the current master
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:18
@keflavich #2658 does...
Adam Ginsburg
@keflavich
Sep 10 2014 10:18
also, @astrofrog, I need the in_ipynb functionality within astroquery: there are times it requests a password from the user, but the password prompt can only be displayed in the terminal, not in the notebook
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:19
@keflavich Apologies, #2789 is the one that takes care of this...
Adam Ginsburg
@keflavich
Sep 10 2014 10:19
@astrocaribe - #2658 is just the issue identifying the problem, right? #2789 is your fix to that problem... ahh, ok, you just said that =)
the question we're dealing with now is how to ensure that the functionality in #2789 gets used "automatically" when a progressbar is invoked in a script
Thomas Robitaille
@astrofrog
Sep 10 2014 10:20
yes
So I think what we need to test is what happens if you try and use the interactive progress bar from the qt console connected to a notebook
If it crashes maybe we can just catch the exception and use the non-interactive progress bar
Adam Ginsburg
@keflavich
Sep 10 2014 10:21
I like that idea
FYI, this: https://github.com/astropy/astroquery/blob/master/astroquery/eso/core.py#L152 is where I need the same functionality in astroquery
Thomas Robitaille
@astrofrog
Sep 10 2014 10:22
My suggestion would be to rename in_ipynb to in_ipynb_kernel to make it clearer what it does
Adam Ginsburg
@keflavich
Sep 10 2014 10:22
ahh, yeah, good idea
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:22
@astrofrog @keflavich This is a very interesting conversation here... I'm catching some of it, but I'd be lying if I said I knew how to fix that :smile:
Thomas Robitaille
@astrofrog
Sep 10 2014 10:23
@astrocaribe - we don't either! ;) Anyway, this will be done independently of astropy/astropy#2789
Stuart Mumford
@Cadair
Sep 10 2014 10:23
@astrofrog you get the email?
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:23
Got it! I'll keep tracking to see what y'all come up with; it's an interesting problem to me!
Thomas Robitaille
@astrofrog
Sep 10 2014 10:24
@Cadair - yes, will see if I'm able to go
Tommy Le Blanc
@astrocaribe
Sep 10 2014 10:25
Well good day guys, time to call it a day (night)... @astrofrog @keflavich Thanks again for all you help and insight!
Adam Ginsburg
@keflavich
Sep 10 2014 10:25
good work @astrocaribe, thanks and goodnight
Thomas Robitaille
@astrofrog
Sep 10 2014 10:25
Thanks @astrocaribe!
@keflavich - very interesting finding: it turns out that if you try and use the progress bar in the console connected to the notebook it doesn't work anyway (even non-interactive) - in master, because it thinks it is not a tty
so if we use the is_ipynb as-is, we don't break anything
tbh, the case of connecting to a kernel with a notebook is pretty rare
@keflavich - it looks like there are some PEP8 failures too. Anyway, once #2789 is merged, we can rebase #2793 and update the progress bar to use is_ipynb
I think we should just go ahead with it as-is, provided it works with ipython 1.0 and 2.0
Thomas Robitaille
@astrofrog
Sep 10 2014 10:31
off for lunch, back later!
Adam Ginsburg
@keflavich
Sep 10 2014 10:32
ok
sorry, missed some of those
@astrofrog - I'll have a look at the pep8 issues
Thomas Robitaille
@astrofrog
Sep 10 2014 11:07
Sounds good