These are chat archives for astropy/astropy

18th
Jun 2015
Christoph Deil
@cdeil
Jun 18 2015 17:41
am I alone?
Matt Craig
@mwcraig
Jun 18 2015 18:22
Mostly....
Wolfgang Kerzendorf
@wkerzendorf
Jun 18 2015 18:22
who knows
Adrian Price-Whelan
@adrn
Jun 18 2015 19:38
I thought the question was: "is there anybody out there?"
Anyways @bmorris3 and @jberlanga -- I'll be around if you need to chat about anything in particular
Brett M. Morris
@bmorris3
Jun 18 2015 19:44
Thanks @adrn . Do you have thoughts on making FixedTarget a subclass of SkyCoord? Discussing with @cdeil
Christoph Deil
@cdeil
Jun 18 2015 19:52
Is there a way to see who's on Gitter at the moment (or at least a count, to see if anyone is here)?
Brett M. Morris
@bmorris3
Jun 18 2015 19:54
A quick look through the Gitter support pages shows no obvious way
Adrian Price-Whelan
@adrn
Jun 18 2015 19:56
@cdeil That seems a bit crazy...
Christoph Deil
@cdeil
Jun 18 2015 19:57
I'm crazy for wanting to know if there's anybody out there before typing a question?
Or Gitter is crazy for not indicating who's online?
Adrian Price-Whelan
@adrn
Jun 18 2015 19:58
gitter is crazy for not making that feature number 2 that they implemented hah
This message was deleted
Brett M. Morris
@bmorris3
Jun 18 2015 19:59
Does it matter if you’re in Lurk Mode?
I’m asking about this in Gitter’s Gitter
Adrian Price-Whelan
@adrn
Jun 18 2015 20:04
:+1:
Brett M. Morris
@bmorris3
Jun 18 2015 20:05

@adrn when you say

there is a lot of magic that is probably better not understood for the time being…

do you mean that it’s too complicated to explain why it’s a bad idea to subclass?

Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:13
@adrn Having a hard time figuring out what's up with my rebasing. I tried to resolve a conflict in .../docs/index.rst, and thought I had done so--I edited the file, removed all conflict markers, saved, did git commit, then git rebase --continue, but now i'm getting the following error:

`Applying: Added skeleton source code (core.py); Modified index.rst.
No changes - did you forget to use 'git add'?
If there is nothing left to stage, chances are that something else
already introduced the same changes; you might want to skip this patch.

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".`

Adrian Price-Whelan
@adrn
Jun 18 2015 20:14
@bmorris3 Sorry, I should be more explicit: on top of the fact that I agree with @cdeil 's notion that a subclass doesn't feel quite right (in an is a sense), I just mean that there is a lot of code in the SkyCoord initializer dealing with parsing inputs and it might be a headache to figure out how to pull out things specific to Target (since we will probably want to accept new kwarg options)
@jberlanga Ah, yes, rebasing can be annoying -- first: have you made a copy of the repo and are experimenting in the copy?
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:15
@adrn aaaaand it looks like I somehow deleted my version of index.rst
@adrn yeah, i made a copy
Adrian Price-Whelan
@adrn
Jun 18 2015 20:17
good, ok, phew -- did it take a lot of work to resolve the conflicts? it might be easier to trash the copy and start over, and go line by line for what you should do?
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:17
let me try again
Brett M. Morris
@bmorris3
Jun 18 2015 20:17
I’ll wait for you two to finish the rebase before I bother @adrn some more. Fun fact — if you scroll up to April in this room, you’ll find Tom helping me do a rebase. Ugh.
Adrian Price-Whelan
@adrn
Jun 18 2015 20:18
oof, that looks way worse than what we're going to have to resolve here :)
Brett M. Morris
@bmorris3
Jun 18 2015 20:22
It felt a bit like a GSoC audition.
Adrian Price-Whelan
@adrn
Jun 18 2015 20:22
@jberlanga from what it sounds like you did, I think you can do the same thing as before, the only change is that when you fix the conflict you don't actually commit the change, you just stage the change with git add, then do git rebase --continue
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:25
ok, i'll try that
Adrian Price-Whelan
@adrn
Jun 18 2015 20:26
i have a meeting 4:30-5 EDT but I'll be back to check in around 5 EDT
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:26
alright, thanks!
Adrian Price-Whelan
@adrn
Jun 18 2015 20:26
@bmorris3 did you want to chat about other stuff? maybe later?
Brett M. Morris
@bmorris3
Jun 18 2015 20:27
@adrn I’m interesting in learning about good design for astropy, so I’d love to have some more comments about why this keyword skimming solution is a bad idea. (I’m not trying to bully you into agreeing with me, I wanna learn :D)
I think part of the question that needs to be answer is - what does FixedTarget do that SkyCoord does not? Because based on the API right now, the answer is: nothing.
Christoph Deil
@cdeil
Jun 18 2015 20:29
Good night and good luck. :sleeping:
I'll probably not be around much on Gitter in the coming weeks, because I'm very bad at having a chat open and getting something done. But @bmorris3 , @jberlanga if there's something you want to discuss or help with, we can talk on Google hangout or Gitter or pair-code sometime ...
Brett M. Morris
@bmorris3
Jun 18 2015 20:29
It stores a name, and we intend to be able to store magnitudes and ephemerides. To me, that still sounds like a handfull of keywords that can be skimmed off a subclass of SkyCoord.
Night @cdeil!
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:30
@cdeil will do!
Adrian Price-Whelan
@adrn
Jun 18 2015 20:30
One issue is that some design choices (like this one) are a little subjective and depend on how much Python kool-aid yer drinking, but some reading:
Namespaces are one honking great idea -- let's do more of those!
This reddit thread
ok i'm off for now, but back later...
(the issue is deeper than just namespaces, but that's a place to start)
Brett M. Morris
@bmorris3
Jun 18 2015 20:40
When you get back, I don’t really see from those examples what namespace has to do this – is it just that FixedTarget(ra, dec).coord.ra is more explicit about what it’s grabbing an RA from than FixedTarget(ra,dec).ra?
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:48
@adrn So, it looks like when I open up index.rst to resolve the conflict, make changes, save, then exit my text editor, all of my changes to the file disappear and I only get back the version of index.rst in master.
That is, when I open the file again (before doing anything else), I can see that it's no longer the version I just saved.
So presumably that's why it's telling me that there are no changes...although me adding core.py should still be there.
Brett M. Morris
@bmorris3
Jun 18 2015 20:53
Is it possible you’re looking at your backup copy?
That’s a mistake I’ve made before and took a really long time to discover, haha
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 20:55
nice
while I have been known to make that kind of mistake, it's not what's happening here
Brett M. Morris
@bmorris3
Jun 18 2015 21:06
Do you have an opinion in the subclassing conversation @jberlanga ?
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 21:12
@adrn I think i figured out the rebasing (not committing seems to have done the trick
@bmorris3 So, if I understand correctly, you're thinking that it would be better to add a FixedTarget subclass to the appropriate Astropy module rather than creating something mostly redundant in astroplan?
Adrian Price-Whelan
@adrn
Jun 18 2015 21:19
back -- @jberlanga so the rebase worked?
@bmorris3 2 issues here:
  1. Do we subclass?
  2. If we don't subclass, how should coordinate features be exposed to user.
Links about namespaces were for 2.
Brett M. Morris
@bmorris3
Jun 18 2015 21:36
Welcome back! How would you expose coordinate features to user without subclass? Like FixedTarget(ra, dec).coord?
Adrian Price-Whelan
@adrn
Jun 18 2015 21:38

About the need for a Target class: I think it is worth having a class for exactly the properties you mention -- e.g., name, magnitudes, epoch, ephemerides, and possibly more features that we may add down the line. We could also add convenience methods for making plots (like airmass curves).

About subclassing: A Target has a coordinate, but a Target is not a coordinate. Most relevant here are discussions about object composition vs. inheritance -- I'm arguing for composition. Here are some links to discussions about the differences:
http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance
http://www.artima.com/designtechniques/compoinh.html
http://www.thoughtworks.com/insights/blog/composition-vs-inheritance-how-choose
(I can only vouch for the stackoverflow link, but the others look nice)

Another specific example of why I think composition is a better idea: a MovingTarget (or whatever name), e.g., an asteroid, should not inherit from SkyCoord, which represent a single place at a specific time.

First off, I would write:

c = SkyCoord(ra=ra, dec=dec, frame='icrs')
targ = FixedTarget(coord=c, ...more stuff)

(e.g., don't allow passing ra and dec in to the initializer)

But then yea, the coordinate stuff is self-contained in a .coord (.coords? .coordinates?) attribute, e.g. if you really need the RA:

targ.coord.ra
Brett M. Morris
@bmorris3
Jun 18 2015 21:42
That first SO link is helpful. I favored the word .coord since coordinates is the name of a module.
Adrian Price-Whelan
@adrn
Jun 18 2015 21:43
BTW: composition vs. inheritance is a subtle (and possible subjective) topic, but I try to stick to the "favor composition over inheritance" viewpoint unless it clearly makes sense to subclass
Brett M. Morris
@bmorris3
Jun 18 2015 21:43
Understood, I see where you’re coming from now.
I need more exposure to Python cultural preferences like this.
Adrian Price-Whelan
@adrn
Jun 18 2015 21:46
this actually goes back to, e.g., java -- it comes up in any OO language
Brett M. Morris
@bmorris3
Jun 18 2015 21:47
It’s easy to live in procedural land as an astronomer, so I guess any OO exposure is good.
Adrian Price-Whelan
@adrn
Jun 18 2015 21:47
but yea, unfortunately it's not something usually taught to scientists who start programming! but that's because we suck at OO
(we = all scientists, joking)
Brett M. Morris
@bmorris3
Jun 18 2015 21:47
There’s such a big chasm between knowing how to code with objects and knowing how to design good ones.
Adrian Price-Whelan
@adrn
Jun 18 2015 21:49
it's true! but this is the best way to pick up these subtleties -- to start hacking and designing API's with other people
Brett M. Morris
@bmorris3
Jun 18 2015 21:49
(hense my nagging for explanations)
Adrian Price-Whelan
@adrn
Jun 18 2015 21:50
please keep doing that! i'm sorry i forget to explain -- this is a learning experience for me as well!
(also it's not nagging)
Adrian Price-Whelan
@adrn
Jun 18 2015 22:03
@bmorris13 And glad you found the mention of set_ / get_ methods in the coding guidelines -- would it be useful to link you to articles / discuss that concept? Using properties instead of setters/getters is definitely a Python cultural preference and, again, could be subjective
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 22:06
@adrn Looks the rebase indeed worked.
Brett M. Morris
@bmorris3
Jun 18 2015 22:06
@adrn I’m familiar with that crusade, actually. The line in the code was a left over from Eric’s original API which got tweaked but not removed.
Jazmin Berlanga Medina
@jberlanga
Jun 18 2015 22:07
@adrn I'm out for now, but I will be back on later tonight and tomorrow (and probably with some plotting questions).
Adrian Price-Whelan
@adrn
Jun 18 2015 22:07
@jberlanga awesome! thanks!
Brett M. Morris
@bmorris3
Jun 18 2015 22:08
Night @jberlanga !
@adrn New question! Say I’m writing tests in the astropy/tests dir, and I want to do a relative import to get core.py. How should I write that? The absolute_import option has me confused
Adrian Price-Whelan
@adrn
Jun 18 2015 22:09
Ah, I think you want from ..core import whatever
Brett M. Morris
@bmorris3
Jun 18 2015 22:10
If i want to import core, can i do from .. import core?
Adrian Price-Whelan
@adrn
Jun 18 2015 22:10
right, yea
@bmorris3 do you want me to merge this skeleton code PR ASAP so you can start hacking in there? I'm fine merging it immediately, but if you aren't going to start working for a day or two, I'll leave it open to see if any of the other astroplanners have comments
Brett M. Morris
@bmorris3
Jun 18 2015 22:12
Doing that gives me ValueError: Attempted relative import in non-package
It’d be great if we could merge and start hacking, I’ve been coding up the guts of Observer and FixedTarget and I’d like to be building into the existing docstrings and updating keywords where relevant
Adrian Price-Whelan
@adrn
Jun 18 2015 22:16
just making sure I understand: are you working in the astroplan repo? does the directory structure look like this? are you working from a file where test_whatever.pyis?
astroplan
|-- __init__.py
|-- core.py
|-- tests
|    |-- __init__.py
|    |-- test_whatever.py
Brett M. Morris
@bmorris3
Jun 18 2015 22:17
That’s correct
Adrian Price-Whelan
@adrn
Jun 18 2015 22:19
Hm, from .. import coreshould work -- how are you running the test_whatever script?
That is, are you using py.test or python setup.py test or are you trying to run the script directly via python astroplan/tests/test_whatever.py?
Brett M. Morris
@bmorris3
Jun 18 2015 22:22
I’m running it directly
It’s just a script with some asserts
Adrian Price-Whelan
@adrn
Jun 18 2015 22:23
ah ok that's the issue -- the relative imports won't work what way. let me think of the clearest way to explain...
the relative imports only work within packages -- by running the script by itself, python doesn't know that it is part of the astroplan package. what you want to do instead is use python setup.py test, which runs all of the tests by calling them in package style, e.g., roughly by calling astroplan.tests.test_whatever.test_name()

just calling python setup.py test will run all of the tests -- you probably just want to run tests in that particular module, or maybe even a subset of the tests in that module. You can use command line args to filter which tests to run. Imagine in my test_whatever.py module, I have 2 tests: one called test_stuff and one called test_more_stuff. I can run:

python setup.py test -t astroplan/tests/test_whatever.py

to run all tests in that module, or

python setup.py test -t astroplan/tests/test_whatever.py --args="-k stuff"

to run all tests that contain the word stuff, or

python setup.py test -t astroplan/tests/test_whatever.py --args="-k more_stuff"

to just run the test called test_more_stuff

Adrian Price-Whelan
@adrn
Jun 18 2015 22:29
(anything passed in to --args="..." is passed through to the call to pytest, so any arguments that work with py.test would work there)
Brett M. Morris
@bmorris3
Jun 18 2015 22:30
Got it. This is more complicated than I wished.
Adrian Price-Whelan
@adrn
Jun 18 2015 22:31
tell me about it ugh
Brett M. Morris
@bmorris3
Jun 18 2015 22:33
I’m just going to keep using my tests.py script in the top-level directory until PR time…
Adrian Price-Whelan
@adrn
Jun 18 2015 22:35
that's fine -- it's a bit more complicated with the astropy core because you want to make sure the tests run on the development version vs. any version you may have installed, but we're a ways off from installing astroplan so I don't think there will be any path clashes!
Brett M. Morris
@bmorris3
Jun 18 2015 22:36
right. plus I’m downloading remote data, so there’s another command line arg… who needs all that.
Adrian Price-Whelan
@adrn
Jun 18 2015 22:39
you do what is easiest / most efficient for you right now since it is still very exploratory (but yea, eventually we will need to move to the standard test format)
Brett M. Morris
@bmorris3
Jun 18 2015 22:40
Great.
Adrian Price-Whelan
@adrn
Jun 18 2015 22:49
I merged the skeleton code in -- hack away! I'll be back on later but gotta run for a bit
Brett M. Morris
@bmorris3
Jun 18 2015 22:50
Thanks!
Adrian Price-Whelan
@adrn
Jun 18 2015 22:50
having flashbacks to chatrooms...
Brett M. Morris
@bmorris3
Jun 18 2015 22:50
I feel some nostolgic brb and g2g’s coming back
Adrian Price-Whelan
@adrn
Jun 18 2015 22:50
g2g cya ttyl