dr-rodriguez / DSII_LocalGroupDB

The Local Group Galaxy Database
4 stars 2 forks source link

Example for presentation #20

Closed eteq closed 4 years ago

eteq commented 4 years ago

This now has an "science case", @dr-rodriguez, using the data from the existing table. I had to make some changes/enhancements to the Python code to realize what I was hoping to do, so you'll probably want to especially look at those commits, @dr-rodriguez. If there's something I was missing about how it was meant to be used we can always back them out, as I tried to keep the changes to the code as distinct commits from the notebook itself.

Note there's one spot near the end that says "[Go to Github and create/merge the PR]". The idea is that while presenting this, this is where we would make a PR to add new data. But the data to be added is also already in this PR. So we'll probably want to make a follow-on commit that just deletes the second stellar_radial_velocity_dispersion measurement for Bootes I, and then we'll make the PR to add it in as part of the presentation.

dr-rodriguez commented 4 years ago

I'll have a look at this in the next few days. Didn't realize I had missed the sign for Dec, but my generation script was meant as a temporary implementation while I waited for the full dataset. I'm surprised you did away with the pandas DataFrame intermediary step- when I tried doing that the code just didn't work due to missing columns. I'll have to run my tests and see what the result is now.

In my personal opinion, I consider this a working prototype and that we are at a stage were we should think about tearing it down and starting from scratch with the lessons learned- though that can wait until after the presentation.

dr-rodriguez commented 4 years ago

I briefly ran through your notebook. I have to update my matplotlib since animation wasn't picked up, but otherwise nice figures! Some of my unit tests are failing so I still need to debug that.

When you added the extra measurement for Bootes (I):

-to specify the right selection, what you want to use is: make_rM_plot(lgaldb.query_table(selection={'stellar_radial_velocity_dispersion': 'Koposov_2011'}), 'Bootes (I)') That tells query_table that the reference for stellar_radial_velocity_dispersion should be Koposov_2011 (or Martin_2007). I'll need to make that clearer in the Tutorial

eteq commented 4 years ago

I'm surprised you did away with the pandas DataFrame intermediary step- when I tried doing that the code just didn't work due to missing columns

Huh, that's odd. It just worked out of the box for me. Maybe it's something in the latest version of Astropy or something?

In my personal opinion, I consider this a working prototype and that we are at a stage were we should think about tearing it down and starting from scratch with the lessons learned- though that can wait until after the presentation.

Yeah, agreed, although making this notebook made me realize this is actually useful as it stands, so it's less clear that tearing it down is called for. But as you say we can discuss that later.

to specify the right selection

Aha! Thanks, yeah, I wasn't really sure I was doing that right - I'll update it as you suggest here and push up another commit.

eteq commented 4 years ago

Oh wait, I just realized you already pushed up commits to update the notebook @dr-rodriguez. Yep looks good to me as it stands now!

dr-rodriguez commented 4 years ago

Shall I go ahead and merge this or did you want to remove the additions to Bootes (I)? From Iva's email, it looks like we won't a lot of time for a presentation/demo.