search

ixc / python-edtf

MIT License
53 stars 19 forks source link

The field _could_ be used with `null=True` #62

Open aweakley opened 5 months ago

aweakley commented 5 months ago

Here my code assumes that natural_text will never be None, https://github.com/ixc/python-edtf/blob/fe96fbdafd77d18ef4f93ed03e9ce5000827c79c/edtf/fields.py#L138 but I think if for some reason you decided to use null=True on your field then it could be None.

ColeDCrawford commented 5 months ago

Somewhat related: I updated a project that was using EDTF v4 to test out the v5 branch and hit this error:

  File "<...>/site-packages/edtf/fields.py", line 123, in update_values
    direct_input = getattr(instance, self.direct_input_field, "")
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: attribute name must be string, not 'NoneType'

This is because I didn't have a direct_input_field defined. Is it reasonable to assume that implementers will have that field, or should we update the fields.py to handle this more gracefully? We should at least call it out in the readme and release notes.

aweakley commented 5 months ago

I've made a PR https://github.com/ixc/python-edtf/pull/68 to add some Django checks for the field.