Skip to content

Remove legacy incertae sedis - #333

Merged
lmrodriguezr merged 3 commits into
mainfrom
remove-legacy-incertae-sedis
Sep 8, 2026
Merged

Remove legacy incertae sedis#333
lmrodriguezr merged 3 commits into
mainfrom
remove-legacy-incertae-sedis

Conversation

@vangberg

Copy link
Copy Markdown
Contributor

Draft – #332 needs to be merged first.

We still have the legacy Name#incertae_sedis field in the database. In preparation for #173 I would like to remove it to reduce the number of edge cases we need to handle.

I have investigated the current state of Name#incertae_sedis in the production database:

There are 16 names with legacy incertae_sedis set:

irb(main):083> nx = Name.where.not(incertae_sedis: [nil, ""])
irb(main):084> nx.size
=> 16

All 16 records have a placement with either a parent or incertae_sedis, which can be verified by running the following:

Name.where.not(incertae_sedis: [nil, ""]).map {|n| [n[:incertae_sedis], n.placement.try(:parent).try(:name), n.placement.try(:incertae_sedis)]}

I interpret that as it being safe to drop this column from the database.

@vangberg
vangberg requested a review from lmrodriguezr August 27, 2026 10:54
@vangberg vangberg self-assigned this Aug 27, 2026
@vangberg
vangberg marked this pull request as draft September 3, 2026 09:28
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from 6ab1e49 to 45079be Compare September 3, 2026 09:35
@vangberg
vangberg force-pushed the direct-batch-placements branch from d85f599 to 09057f0 Compare September 3, 2026 09:35
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from 45079be to 7865546 Compare September 3, 2026 09:48
@vangberg
vangberg force-pushed the direct-batch-placements branch 2 times, most recently from 4a49fd4 to 09057f0 Compare September 3, 2026 11:54
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from 7865546 to bf35ee5 Compare September 3, 2026 11:57
@vangberg
vangberg force-pushed the direct-batch-placements branch 2 times, most recently from 2304b7b to f967e96 Compare September 3, 2026 12:12
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from bf35ee5 to a537e97 Compare September 3, 2026 12:12
@vangberg
vangberg force-pushed the direct-batch-placements branch from f967e96 to 7281549 Compare September 3, 2026 12:28
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from a537e97 to 308f14d Compare September 3, 2026 12:30
@vangberg
vangberg removed the request for review from lmrodriguezr September 3, 2026 12:33
@vangberg
vangberg force-pushed the direct-batch-placements branch from 7281549 to 145ea82 Compare September 3, 2026 12:39
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from 308f14d to 5630d05 Compare September 3, 2026 12:43
@vangberg
vangberg force-pushed the remove-legacy-incertae-sedis branch from 5630d05 to 73966e2 Compare September 3, 2026 12:46
@vangberg
vangberg changed the base branch from direct-batch-placements to rank-aware-name-autocomplete September 3, 2026 12:48
name = Name.find_by_variants(par['name'])
name.update!(new_par)
name.update_column(:incertae_sedis, nil) if par['incertae_sedis']
if placement_par

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to review if we can do this a bit simpler/re-use existing code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not great, but I think it is easier to handle it after we are done with the incertae sedis-related changes. Then we can either go with the Name#preferred_placement proposal or implement some shared Name#set_preferred_placement method.

@vangberg vangberg left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmrodriguezr Ready for review.

There are 16 name records in production with non-nil incertae_sedis. They all have at least one placement record, which means that the incertae_sedis from Name is never read, so I think it is safe to remove this column.

irb(main):008> nx = Name.where.not(incertae_sedis: [nil, ""])
=> 
[#<Name:0x0000735e803c7da8
...
irb(main):009> nx.size
=> 16
irb(main):010> nx.map {|n| n.placement.present?}
=> [true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true]

name = Name.find_by_variants(par['name'])
name.update!(new_par)
name.update_column(:incertae_sedis, nil) if par['incertae_sedis']
if placement_par

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not great, but I think it is easier to handle it after we are done with the incertae sedis-related changes. Then we can either go with the Name#preferred_placement proposal or implement some shared Name#set_preferred_placement method.

@vangberg
vangberg marked this pull request as ready for review September 8, 2026 08:48
@vangberg
vangberg changed the base branch from rank-aware-name-autocomplete to main September 8, 2026 08:52
@lmrodriguezr
lmrodriguezr merged commit 8d45bd8 into main Sep 8, 2026
1 check passed
@lmrodriguezr
lmrodriguezr deleted the remove-legacy-incertae-sedis branch September 8, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants