UUDigitalHumanitieslab / I-analyzer

The great textmining tool that obviates all others
https://ianalyzer.hum.uu.nl
MIT License
6 stars 2 forks source link

Improve documentation of `addcorpus.models.Field.language` #1522

Open BeritJanssen opened 3 months ago

BeritJanssen commented 3 months ago

The language field of the Field model looks strange to me: It has a CharField with blank=True, null=False. So then we should set a default, I believe? NB null=False is not necessary to state explicitly (and it's not advised to set this to True on CharFields anyway). validate_field_language seems to assume that fields which don't have an explicit language tag set would have to have "dynamic" set. I'm not sure I agree with enforcing that every field, or for "dynamic", the corpus, has a valid language tag, in some corpora we simply don't have that kind of metadata. A docstring in validate_field_language about the meaning of the "dynamic" setting may also not be misplaced.

lukavdplas commented 3 months ago
BeritJanssen commented 3 months ago
  • For CharFields (at least with null = False and blank = True), '' is already the default, so you don't need to set it explicitly.

Ah! So that's indeed a default. Hadn't stumbled upon that yet in the Django documentation. I do think that if we are to explicitly state values, blank=True, default='' is more readable.