Feature: new frame model #46

talister · 2015-10-29T19:38:56Z

Hi Sarah (and Edward), I think I'm done with these changes to the Frame model/table, please review.

Replace old partial migration with complete one. Lengthen some fields and (hopefully) get the right combination of null/blank... Modify the __unicode__ to handle the case of no filename.

sgreenstreet · 2015-10-29T22:31:34Z

neoexchange/core/models.py

+    exptime     = models.FloatField('Exposure time in seconds', null=True, blank=True)
+    midpoint    = models.DateTimeField('UTC date/time of frame midpoint', null=False, blank=False)
+    block       = models.ForeignKey(Block, null=True, blank=True)
+    quality     = models.IntegerField('Frame Quality', help_text='Frame Quality (-1: unassessed)', default=-1)


The quality would probably be better as a string of comma-separated characters than an integer.

On Thu, Oct 29, 2015 at 3:31 PM, sgreenstreet notifications@github.com
wrote:

In neoexchange/core/models.py
#46 (comment):

filter = models.CharField('filter class', max_length=15)

filename = models.CharField(max_length=31)

exp = models.FloatField('exposure time in seconds')

whentaken = models.DateTimeField()

block = models.ForeignKey(Block)

filename = models.CharField('FITS filename', max_length=40, blank=True)

exptime = models.FloatField('Exposure time in seconds', null=True, blank=True)

midpoint = models.DateTimeField('UTC date/time of frame midpoint', null=False, blank=False)

block = models.ForeignKey(Block, null=True, blank=True)

quality = models.IntegerField('Frame Quality', help_text='Frame Quality (-1: unassessed)', default=-1)

The quality would probably be better as a string of comma-separated
characters than an integer.

—
Reply to this email directly or view it on GitHub
https://github.com/LCOGT/neoexchange/pull/46/files#r43452974.

Changing. Does the ADES new MPC format spec list a maximum length for this
quality/notes field?

Dr Tim Lister
Astronomer and Project Scientist
Las Cumbres Observatory (LCOGT.net)
+1 805 880 1989

It does not state a maximum length. It simply says the notes can be a set of one-character flags to communicate observing circumstances.

@sgreenstreet

…ed by @sgreenstreet

sgreenstreet · 2015-10-29T23:24:20Z

This looks good to me. The unit tests passed when I ran them. I just had a couple minor comments. I'm a little hazy on the admin part, however. I tried to add a new frame into the admin page through running the server, but ran into an error (OperationalError at /admin/core/frame/add/; no such table: ingest_frame). I'm not sure where this is coming from.

talister · 2015-10-29T23:26:54Z

On Thu, Oct 29, 2015 at 4:24 PM, sgreenstreet notifications@github.com
wrote:

This looks good to me. The unit tests passed when I ran them. I just had a
couple minor comments. I'm a little hazy on the admin part, however. I
tried to add a new frame into the admin page through running the server,
but ran into an error (OperationalError at /admin/core/frame/add/; no such
table: ingest_frame). I'm not sure where this is coming from.

—
Reply to this email directly or view it on GitHub
#46 (comment).

You need to stop the development server, run 'python manage.py migrate' to
get it to build the new table and then restart the dev. server with 'python
manage.py runserver'. Please try this out as I am a little worried that the
migrations might not be doing the right thing (I have a somewhat odd setup
now on my test server due to part-failed previous versions of the migration)

Dr Tim Lister
Astronomer and Project Scientist
Las Cumbres Observatory (LCOGT.net)
+1 805 880 1989

sgreenstreet · 2015-10-29T23:36:12Z

Alright, I did the migration and restarted the dev. server and everything appears to be working just fine. The unit tests still pass as well.

talister · 2015-10-29T23:39:00Z

Over to Edward for review/sign-off

zemogle · 2015-10-30T00:05:26Z

neoexchange/core/models.py

-    site        = models.CharField('3-letter site code', max_length=3)
-    instrument  = models.CharField('instrument code', max_length=4)
+    sitecode    = models.CharField('MPC site code', max_length=4)
+    instrument  = models.CharField('instrument code', max_length=4, blank=True)


If you want this to not need to be added to the db you will need to add null=True as well.

On Thu, Oct 29, 2015 at 5:05 PM, Edward notifications@github.com wrote:

In neoexchange/core/models.py
#46 (comment):

'''

site = models.CharField('3-letter site code', max_length=3)

instrument = models.CharField('instrument code', max_length=4)

sitecode = models.CharField('MPC site code', max_length=4)

instrument = models.CharField('instrument code', max_length=4, blank=True)

If you want this to not need to be added to the db you will need to add
null=True as well.

—
Reply to this email directly or view it on GitHub
https://github.com/LCOGT/neoexchange/pull/46/files#r43459886.

I thought Austin was against null=True for CharFields?

Dr Tim Lister
Astronomer and Project Scientist
Las Cumbres Observatory (LCOGT.net)
+1 805 880 1989

I wasn't sure what his point was. I think as is, you'll never be able to have the field empty.

On Thu, Oct 29, 2015 at 5:09 PM, Edward notifications@github.com wrote:

In neoexchange/core/models.py
#46 (comment):

'''

site = models.CharField('3-letter site code', max_length=3)

instrument = models.CharField('instrument code', max_length=4)

sitecode = models.CharField('MPC site code', max_length=4)

instrument = models.CharField('instrument code', max_length=4, blank=True)

I wasn't sure what his point was. I think as is, you'll never be able to
have the field empty.

—
Reply to this email directly or view it on GitHub
https://github.com/LCOGT/neoexchange/pull/46/files#r43460173.

sitecode should never be empty (we'll always have one of those) but
instrument can be - I think this is tested and proved in the unittests.

Dr Tim Lister
Astronomer and Project Scientist
Las Cumbres Observatory (LCOGT.net)
+1 805 880 1989

I have a memory that unittests run on SQLite which accept this, whereas MySQL might not.

…elds

talister · 2015-10-30T16:54:11Z

Running on my development version with a real MySQL backend, creating a blank Frame in the admin and trying to save it result in errors for the missing site code, filter and midpoint. Filling those in allows the creation of a Frame entry with everything else blank/null. So I think that is working correctly. I have also added some more unittests to check this.

Feature: new frame model

zemogle · 2015-10-30T18:53:14Z

Do we want to push it to dev today?

talister · 2015-10-30T19:59:23Z

On Fri, Oct 30, 2015 at 11:53 AM, Edward notifications@github.com wrote:

Do we want to push it to dev today?

It might be worth trying, but mainly to see if the migrations apply
properly (I've been having a lot of trouble with them on the
feature-source_measurements branch). There's no user-visible functionality
difference until I get the rest of the feature-source_measurements branch
done.

—

Reply to this email directly or view it on GitHub
#46 (comment).

Tim

Dr Tim Lister
Astronomer and Project Scientist
Las Cumbres Observatory (LCOGT.net)
+1 805 880 1989

talister added 4 commits October 26, 2015 18:24

Rename Record model to Frame, adjust admin

9c2044f

Merge branch 'master' into feature-new_frame_model

b652c9b

Complete fillout of fields for Frame model and add unit tests.

be6e86d

Replace old partial migration with complete one. Lengthen some fields and (hopefully) get the right combination of null/blank... Modify the __unicode__ to handle the case of no filename.

Update admin class for Frame to show more useful things

d3223b5

talister added the enhancement label Oct 29, 2015

talister assigned sgreenstreet Oct 29, 2015

sgreenstreet reviewed Oct 29, 2015
View reviewed changes

Change quality from an Int to a Char. Fix tests and extra one suggest…

25a4143

…ed by @sgreenstreet

talister assigned zemogle and unassigned sgreenstreet Oct 29, 2015

zemogle reviewed Oct 30, 2015
View reviewed changes

Add tests that check IntegrityErrors are raised for missing needed fi…

288c616

…elds

talister added a commit that referenced this pull request Oct 30, 2015

Merge pull request #46 from LCOGT/feature-new_frame_model

198bac4

Feature: new frame model

talister merged commit 198bac4 into master Oct 30, 2015

talister deleted the feature-new_frame_model branch October 30, 2015 17:03

talister mentioned this pull request Nov 4, 2015

Download and store prior source measurements #52

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: new frame model #46

Feature: new frame model #46

talister commented Oct 29, 2015

sgreenstreet Oct 29, 2015

talister Oct 29, 2015

sgreenstreet Oct 29, 2015

sgreenstreet commented Oct 29, 2015

talister commented Oct 29, 2015

sgreenstreet commented Oct 29, 2015

talister commented Oct 29, 2015

zemogle Oct 30, 2015

talister Oct 30, 2015

zemogle Oct 30, 2015

talister Oct 30, 2015

zemogle Oct 30, 2015

talister commented Oct 30, 2015

zemogle commented Oct 30, 2015

talister commented Oct 30, 2015

Feature: new frame model #46

Feature: new frame model #46

Conversation

talister commented Oct 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgreenstreet commented Oct 29, 2015

talister commented Oct 29, 2015

sgreenstreet commented Oct 29, 2015

talister commented Oct 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talister commented Oct 30, 2015

zemogle commented Oct 30, 2015

talister commented Oct 30, 2015