Command to remove obsolete field choice objects

[susanodd]

A single management command (remove_fieldchoices) to remove FieldChoice objects of categories not used anymore by the code. The command accepts one or more field category names as arguments (e.g. python manage.py remove_fieldchoices Handshape derivHist SemField), lists the objects that will be deleted, and asks for confirmation before proceeding.

Note, this is a one-off command and on subsequent usage it has no effect.

---

⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

[Woseseltops]

• These 3 commands are quite similar, perhaps they can be one command with the object name as parameter?
• Perhaps show what is about to be deleted, and ask the user to confirm?

[Woseseltops]

@copilot , can you execute my proposals?

[Copilot]

Pull request overview

Adds one-off Django management commands to delete obsolete FieldChoice rows for deprecated categories (SemField, Handshape, derivHist) from the Signbank dictionary app.

Changes:

• Added remove_semField_fieldchoices command to delete FieldChoice(field='SemField').
• Added remove_handshape_fieldchoices command to delete FieldChoice(field='Handshape').
• Added remove_derivHist_fieldchoices command to delete FieldChoice(field='derivHist').

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
signbank/dictionary/management/commands/remove_semField_fieldchoices.py	New command to remove obsolete SemField field choices.
signbank/dictionary/management/commands/remove_handshape_fieldchoices.py	New command to remove obsolete Handshape field choices.
signbank/dictionary/management/commands/remove_derivHist_fieldchoices.py	New command to remove obsolete derivHist field choices.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The module filename contains mixed casing (derivHist). All other management commands in this directory use lowercase snake_case filenames; mixed case makes the manage.py command name case-sensitive/unexpected on Linux. Consider renaming this command module to a lowercase name (e.g., remove_derivhist_fieldchoices) and keep the field value ('derivHist') inside the query.

[Copilot]

The variable name field_choices_for_handshape is misleading here because the queryset is filtering field='derivHist'. Rename it to something accurate/generic (e.g., field_choices_for_derivhist or field_choices) to avoid confusion.

Suggested change

	field_choices_for_handshape = FieldChoice.objects.filter(field__exact='derivHist')
	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	field_choices_for_derivhist = FieldChoice.objects.filter(field__exact='derivHist')
	if field_choices_for_derivhist.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_derivhist:

[Copilot]

count() == 0 performs an extra SELECT COUNT(*) and then the queryset is evaluated again during iteration. Prefer exists() for the empty check, and consider iterating with .iterator() if many rows are expected to avoid caching the full result set in memory.

Suggested change

	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	if not field_choices_for_handshape.exists():
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape.iterator():

[Copilot]

The module filename contains mixed casing (SemField). All other management commands in this directory use lowercase snake_case filenames; mixed case makes the manage.py command name case-sensitive/unexpected on Linux. Consider renaming this command module to a lowercase name (e.g., remove_semfield_fieldchoices) and keep the field value ('SemField') inside the query.

[Copilot]

The variable name field_choices_for_handshape is misleading here because the queryset is filtering field='SemField'. Rename it to something accurate/generic (e.g., field_choices_for_semfield or field_choices) to avoid confusion when running/maintaining this one-off command.

Suggested change

	field_choices_for_handshape = FieldChoice.objects.filter(field__exact='SemField')
	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	field_choices_for_semfield = FieldChoice.objects.filter(field__exact='SemField')
	if field_choices_for_semfield.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_semfield:

[Copilot]

count() == 0 performs an extra SELECT COUNT(*) and then the queryset is evaluated again during iteration. Prefer exists() for the empty check, and consider iterating with .iterator() if many rows are expected to avoid caching the full result set in memory.

Suggested change

	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	if not field_choices_for_handshape.exists():
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape.iterator():

[Copilot]

Same pattern here: count() == 0 runs an extra SELECT COUNT(*) and the queryset is later evaluated again for iteration. Prefer exists() for the empty check, and consider .iterator() if this could delete many rows to avoid caching all results in memory.

Suggested change

	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	if not field_choices_for_handshape.exists():
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape.iterator():

[Copilot]

These three new management commands are almost identical except for the field value. To reduce duplication and keep future cleanup simpler, consider a single command that accepts one or more field names as arguments (or hard-codes a list) and deletes matching FieldChoice rows accordingly.

Suggested change

	help = "Remove obsolete field='Handshape' objects from FieldChoice."
	def handle(self, *args, **options):
	field_choices_for_handshape = FieldChoice.objects.filter(field__exact='Handshape')
	if field_choices_for_handshape.count() == 0:
	print('No objects found matching query')
	return
	for fc in field_choices_for_handshape:
	print("Remove FieldChoice object with field='Handshape', machine_value=", fc.machine_value, ", name=", fc.name)
	help = "Remove obsolete FieldChoice objects for one or more fields (default: field='Handshape')."
	def add_arguments(self, parser):
	"""
	Optionally specify one or more fields to clean up.
	If no fields are provided, the command defaults to field='Handshape'
	to preserve existing behavior.
	"""
	parser.add_argument(
	'-f',
	'--field',
	dest='fields',
	action='append',
	help="Field name to remove obsolete FieldChoice objects for. "
	"Can be specified multiple times.",
	)
	def handle(self, *args, **options):
	fields = options.get('fields') or ['Handshape']
	field_choices = FieldChoice.objects.filter(field__in=fields)
	if field_choices.count() == 0:
	print('No FieldChoice objects found matching fields:', ', '.join(fields))
	return
	for fc in field_choices:
	print(
	"Remove FieldChoice object with field='{}', machine_value={}, name={}".format(
	fc.field, fc.machine_value, fc.name
	)
	)

[Copilot]

• These 3 commands are quite similar, perhaps they can be one command with the object name as parameter?
• Perhaps show what is about to be deleted, and ask the user to confirm?

Done in commit 761b25d. The three separate commands have been merged into a single remove_fieldchoices command that accepts one or more field category names as arguments (e.g. python manage.py remove_fieldchoices Handshape derivHist SemField). Before any deletion it lists the objects that will be removed and asks for confirmation (yes/no).

@copilot , can you execute my proposals?

Both proposals have been executed in commit 761b25d — the three commands are now one (remove_fieldchoices) with the field category name(s) as arguments, and the command lists what will be deleted and asks for confirmation before proceeding.

[susanodd]

• These 3 commands are quite similar, perhaps they can be one command with the object name as parameter?
• Perhaps show what is about to be deleted, and ask the user to confirm?

No, this should not be a parameter. These are only there because they are object models now, not field choices. They were left in the database because of migrations (like 3 or 4 years ago).

The user should not be able to delete an arbitrary field choice category. Deleting specific choices that are in use is prevented in the Admin.

The user should not be allowed to "confirm" or "not confirm" since these are obsolete things being deleted.

[susanodd]

• These 3 commands are quite similar, perhaps they can be one command with the object name as parameter?
• Perhaps show what is about to be deleted, and ask the user to confirm?

@Woseseltops this probably ought to be a MIGRATION to get rid of these.

It doesn't make any sense to make this some kind of general command.

It would be like allowing the user to delete "int" or something. The field choice categories are inside the model field definition. These are meant to remove the categories that are no longer used in the model types.

It makes no sense at all to allow the user a parameter for the category. If they would delete stuff they will make it so Signbank will no longer compile.

I made them commands because this is straightforward. But these ought to be done in a migration. This is not something that's optional. (You can see in the Admin under Field Choice that there are some objects with no category shown. That's because these categories were removed. But the objects were not removed. That is because there were migrations for all of these and the migrations had to map the data. That was around the time of Covid. Long ago. But the objects are hanging around.