Potentially inappropriate not null specifications on wells table columns

[hjw]

Almost every column on the wells table is defined with a NOT NULL flag. This works for varchar and text strings, but dates, booleans and numbers.... at least this is my understanding.

Potentially latitude, longitude, total_depth, depth_to_bedrock, all should be changed in the models file, adding blank=true. Not being sure about this I'm hoping Mark, or some other django guy will chime in.

[bsweger]

Hi Hawley,

blank=true tells Django to accept a blank value when the data is entered via form. To actually change the db schema from NOT NULL to NULL, you also have to update the model definition to read null=True. The exception is anything defined as a CharField or TextField.

[leveille]

And here's some relevant documentation:

https://docs.djangoproject.com/en/dev/ref/models/fields/#null

Avoid using null on string-based fields such as CharField and TextField unless you have an excellent reason. If a string-based field has null=True, that means it has two possible values for “no data”: NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” Django convention is to use the empty string, not NULL.

https://docs.djangoproject.com/en/dev/ref/models/fields/#blank

[leveille]

And I'm certainly not trying to just tell you to RTFD. If I can provide some help here I'm more than willing to do so.

[hjw]

Hey jason, I put the issue in because Tuesday I hit a snag importing the well data from the legacy heroku db… getting errors of the field cannot be null type.

when I encountered the problem I: I checked the sqlite db table's schema, almost all of it's columns were defined to be NOT NULL. Headed to the models.py file for wells, saw the blank=True parameters, read the same docs that you included in your email and concluded that I don't know why we aren't specifying null=True for all of our non-string columns.

On Sunday I was upstairs exploring mapping options on the iMacs in the lab (more screen real-estate) when you were knocking out the django models so I don't know if you/Mark intentionally made most of the non-string fields as required. This is why I put up the issue (possibly poorly worded).

I'd like to make sure that adding null=True to the non-required non-character based fields is the right thing to do instead of just going ahead and doing it. My understanding is that other then a DEP well id# we shouldn't be requiring any other fields because of the incompleteness of the existing data, but maybe I'm wrong. Was there any discussion about required vs not-required for well model fields when you were creating the models?

On Jun 5, 2013, at 9:47 AM, Jason Leveille notifications@github.com wrote:

And I'm certainly not trying to just tell you to RTFD. If I can provide some help here I'm more than willing to do so.

— Reply to this email directly or view it on GitHub.

[leveille]

Just trying to make sure I understand:

getting errors of the field cannot be null type. when I encountered the problem I: I checked the sqlite db table's schema, almost all of it's columns were defined to be NOT NULL.

You'd be getting "field cannot be null" errors if you have NULL definitions on the heroku side, but we have NOT NULL definitions on the django/pgsql side.

Headed to the models.py file for wells, saw the blank=True parameters, read the same docs that you included in your email and concluded that I don't know why we aren't specifying null=True for all of our non-string columns.

From what I can tell, the model definition for wells (https://github.com/hackforwesternmass/frcog-private-wells/blob/frcog-django/frcog/wells/models.py#L25) matches the original schema (https://github.com/hackforwesternmass/frcog-private-wells/blob/master/database/all.sql#L72) ... with the exception of field notes. So (originally), I didn't specify null=True for non-string fields if I didn't see null in the all.sql file. Example:

latitude        double precision,
longitude       double precision,
total_depth         integer,
depth_to_bedrock    integer

And the corresponding definition in Django:

latitude = models.DecimalField(max_digits=11, decimal_places=6)
longitude = models.DecimalField(max_digits=11, decimal_places=6)
total_depth = models.IntegerField()
depth_to_bedrock = models.IntegerField()

It's certainly possible that I screwed something up. Definitely wouldn't be the first time. So, the short answer is, I tried to follow what I saw in all.sql. The only discussion I had was asking about field_notes. I just assumed the discussion of required/not required had already happened prior to the creation of all.sql. So, that's what I followed. Sorry if this is causing additional work/headache for you.

[hjw]

No headache. Just normal communication issues, given the nature of a hackathon.

Your help certainly didn't cause any headaches for me, it took the headaches away and gave us working models.

I do have a question about what you wrote below:

On Jun 5, 2013, at 10:31 AM, Jason Leveille notifications@github.com wrote:

From what I can tell, the model definition for wells (https://github.com/hackforwesternmass/frcog-private-wells/blob/frcog-django/frcog/wells/models.py#L25) matches the original schema (https://github.com/hackforwesternmass/frcog-private-wells/blob/master/database/all.sql#L72) ... with the exception of field notes. So (originally), I didn't specify null=True for non-string fields if I didn't see null in the all.sql file. Example:

latitude double precision, longitude double precision, total_depth integer, depth_to_bedrock integer And the corresponding definition in Django:

latitude = models.DecimalField(max_digits=11, decimal_places=6) longitude = models.DecimalField(max_digits=11, decimal_places=6) total_depth = models.IntegerField() depth_to_bedrock = models.IntegerField() Don't these two result in different table defs? I thought that in an sql column definition the default was not to make the field required, but in django the default is to make the field required, so wouldn't the table defined by the sql result in plain old columns and the django result in required/NOT NULL columns, or do I have this mixed up?

[leveille]

Don't these two result in different table defs? I thought that in an sql column definition the default was not to make the field required, but in django the default is to make the field required, so wouldn't the table defined by the sql result in plain old columns and the django result in required/NOT NULL columns, or do I have this mixed up?

You are 100% correct. I messed that up.

[hjw]

Phew. I'll change the model files this afternoon and push to GitHub.

On Jun 5, 2013, at 11:11, Jason Leveille notifications@github.com wrote:

Don't these two result in different table defs? I thought that in an sql column definition the default was not to make the field required, but in django the default is to make the field required, so wouldn't the table defined by the sql result in plain old columns and the django result in required/NOT NULL columns, or do I have this mixed up?

You are 100% correct. I messed that up.

— Reply to this email directly or view it on GitHub.