Skip to content

Conversation

acwhite211
Copy link
Member

@acwhite211 acwhite211 commented Sep 29, 2025

Fixes #7423

Resolve sqlalchemy compiling issues related to the group_concat and blank_nulls by making their definitions more friendly to sqlachlemy 1.4. The main way to do this is my setting type = String(), which gives the expression a real TypeEngine, so the compiler/comparator path won’t hit None/NullType.

Checklist

  • Self-review the PR after opening it to make sure the changes look good and
    self-explanatory (or properly documented)
  • Add relevant issue to release milestone
  • Add pr to documentation list

UX Testing instructions

  • Use the vpl database.
  • Through the Specify Taxon Tree UI, merge two taxon nodes (e.g., 'Palaeoniscidae (in Palaeonisciformes)' into 'Palaeoniscidae (in Palaeoniscoidea)'). See that the taxon merge occurred without errors.

Running the merge:
Image

Taxon Tree after merge:
Image

  • Try merging two other random taxon nodes in three to make sure no errors occur.

Development Testing instructions

  • Query the database with:

    select count(*)
    from taxon t
    join taxon p on t.parentid = p.taxonid
    where t.nodenumber not between p.nodenumber and p.highestchildnodenumber;
  • See that it returns a count of 0

  • Run the renumber_tree('taxon') function directly via the Django Python shell.

    from specifyweb.backend.trees import extras
    from django.db import connection
    extras.renumber_tree('taxon')
    ❯ docker exec -it specify7 /bin/bash
    specify@4f497e2137b1:/opt/specify7$ ve/bin/python manage.py shell
    Python 3.12.3 (main, Aug 14 2025, 17:47:21) [GCC 13.3.0] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    (InteractiveConsole)
    >>> from specifyweb.backend.trees import extras
    >>> from django.db import connection
    >>> extras.renumber_tree('taxon')
    [15/Sep/2025 12:14:47] [INFO] [specifyweb.specify.tree_extras:643] renumbering tree
    
  • The command renumber_tree('taxon') ran without errors.

  • Query the database with:

    select count(*)
    from taxon t
    join taxon p on t.parentid = p.taxonid
    where t.nodenumber not between p.nodenumber and p.highestchildnodenumber;
  • The query should return 0.

  • Through the Specify Taxon Tree UI, merge two taxon nodes (e.g., 'Palaeoniscidae (in Palaeonisciformes)' into 'Palaeoniscidae (in Palaeoniscoidea)'). See that the taxon merge occurred without errors.

  • Go back to the python shell and run the validate_tree_numbering('taxon') command

from specifyweb.backend.trees import extras
from django.db import connection
extras.validate_tree_numbering('taxon')
  • See that the validate_tree_numbering('taxon') command ran without errors

@acwhite211 acwhite211 marked this pull request as ready for review October 3, 2025 17:00
@acwhite211 acwhite211 requested review from a team October 3, 2025 17:00
@CarolineDenis CarolineDenis added this to the 7.12.0 milestone Oct 6, 2025
@acwhite211 acwhite211 modified the milestones: 7.12.0, 7.11.3 Oct 17, 2025
Copy link
Collaborator

@bhumikaguptaa bhumikaguptaa left a comment

Choose a reason for hiding this comment

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

  • Through the Specify Taxon Tree UI, merge two taxon nodes (e.g., 'Palaeoniscidae (in Palaeonisciformes)' into 'Palaeoniscidae (in Palaeoniscoidea)'). See that the taxon merge occurred without errors.

  • Try merging two other random taxon nodes in three to make sure no errors occur.

'Palaeoniscidae' in the tree was not found to merge, otherwise i was able to perform random taxon merges.

Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Through the Specify Taxon Tree UI, merge two taxon nodes (e.g., 'Palaeoniscidae (in Palaeonisciformes)' into 'Palaeoniscidae (in Palaeoniscoidea)'). See that the taxon merge occurred without errors
  • Try merging two other random taxon nodes in three to make sure no errors occur.

Could not recreate the issue in main but it seems to be working on this branch

@grantfitzsimmons grantfitzsimmons requested a review from a team October 20, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 📋Back Log

Development

Successfully merging this pull request may close these issues.

Tree renumbering function introduces invalid numbering, causing merge failures

5 participants