Skip to content

Move taxonomy model code to models_future - #1499

Open
kra wants to merge 1 commit into
mainfrom
kra/model-split-tests-storage-taxonomy
Open

kra wants to merge 1 commit into
mainfrom
kra/model-split-tests-storage-taxonomy

Conversation

@kra

@kra kra commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Move taxonomy model code to models_future

List of Changes

Moved code from ami.main.models to ami.main.models_future.taxonomy:
- _case_from_map
- verified_taxon_counts
- TaxonRank
- TaxonQuerySet
- TaxonManager
- TaxonParent
- Taxon
- TaxaListQuerySet
- TaxaListManager
- TaxaList
- Tag

Related Issues

Relates to #260

Detailed Description

Moved code from ami.main.models to ami.main.models_future.taxonomy:
- _case_from_map
- verified_taxon_counts
- TaxonRank
- TaxonQuerySet
- TaxonManager
- TaxonParent
- Taxon
- TaxaListQuerySet
- TaxaListManager
- TaxaList
- Tag

Because main.models still has to re-export from models_future until migrations and other code stops referencing it:
- Added imports in some methods, since module-level imports must be avoided to avoid circular imports.
- Changed type hinting for some methods to strings (to be evauluated by the linter without a change in behavior) to avoid circular imports.

Added unit tests for models_future.taxonomy. Some of these were driven by coverage needs. I didn't remove tests from ami.main, which means we have duplicate coverage. The tests were all blatted out by AI and I didn't work hard to understand them.

There should be no functionality changes, including possible bugfixes.

How to Test the Changes

docker compose -f docker-compose.ci.yml run --rm django python manage.py test

Summary by CodeRabbit

  • Refactor
    • Consolidated taxonomy data handling for taxa, lists, tags, and rank defaults. Taxonomy-related counts, visibility, filtering, and tree views continue to be supported, including project-specific defaults and verified-identification counts. This update reorganizes how taxonomy features are provided without changing the available user-facing capabilities.

@kra
kra requested a review from mihow October 8, 2026 22:13
@netlify

netlify Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for antenna-preview canceled.

Name Link
🔨 Latest commit 87c836c
🔍 Latest deploy log https://app.netlify.com/projects/antenna-preview/deploys/6ac968490d227c0008986f05

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 073b1baa-a84f-431a-8fc2-a49dbd8f4239

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5149e and 87c836c.


📒 Files selected for processing (5)
  • ami/main/models.py
  • ami/main/models_future/__init__.py
  • ami/main/models_future/enums.py
  • ami/main/models_future/taxonomy.py
  • ami/main/models_future/tests/test_taxonomy.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

Taxonomy ranks, models, managers, querysets, and helpers now live in the future-model modules, with names re-exported from ami.main.models. New tests cover taxonomy model operations, queryset annotations, filtering, and visibility.

Changes

Taxonomy models

Layer / File(s) Summary
Define taxonomy models and model behavior
ami/main/models_future/enums.py, ami/main/models_future/taxonomy.py, ami/main/models.py, ami/main/models_future/__init__.py, ami/main/models_future/tests/test_taxonomy.py
The future-model modules define rank values, taxonomy models, managers, tree and cache operations, and taxa-list behavior. ami.main.models re-exports taxonomy names. Tests cover model methods, relationships, and calculated fields.
Add taxonomy query and filtering behavior
ami/main/models_future/taxonomy.py, ami/main/models_future/tests/test_taxonomy.py
Querysets calculate observation and verification counts, select example occurrences, and filter taxa by project defaults and user visibility. Tests cover these query paths and filters.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to 87c83

This change reorganizes the taxonomy model code into a new module and keeps the old import paths working. No concrete behavior change or breakage was found, so it looks ready to merge.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 32.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 166 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description includes the summary, change list, related issue, detailed description, testing command, circular-import rationale, and risk context. It omits the optional Screenshots section, Deploym…
Title check Passed The title clearly and concisely describes the primary change: moving taxonomy model code to models_future.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kra
kra added this pull request to stack #1489 October 8, 2026 22:13
@netlify

netlify Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for antenna-ssec canceled.

Name Link
🔨 Latest commit 87c836c
🔍 Latest deploy log https://app.netlify.com/projects/antenna-ssec/deploys/6ac96849e044850008fb91cb

@kra kra changed the title Kra/model split tests storage taxonomy Move taxonomy model code to models_future Oct 8, 2026
Base automatically changed from kra/model-split-tests-storage to main October 9, 2026 21:26
- _case_from_map
- verified_taxon_counts
- TaxonRank
- TaxonQuerySet
- TaxonManager
- TaxonParent
- Taxon
- TaxaListQuerySet
- TaxaListManager
- TaxaList
- Tag

Because main.models still has to re-export from models_future until migrations and other code stops referencing it:
- Added imports in some methods, since module-level imports must be avoided to avoid circular imports.
- Changed type hinting for some methods to strings (to be evauluated by the linter without a change in behavior) to avoid circular imports.

Added unit tests for models_future.taxonomy. Some of these were driven by coverage needs. I didn't remove tests from ami.main, which means we have duplicate coverage. The tests were all blatted out by AI and I didn't work hard to understand them.

There should be no functionality changes, including possible bugfixes.
@kra
kra force-pushed the kra/model-split-tests-storage-taxonomy branch from e4819a3 to 87c836c Compare October 9, 2026 22:18

@mihow mihow Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This one still feels quite big and is going to get bigger! I think taxonomy is worthy of its own app: ami/taxonomy/models/taxon.py ami/taxonomy/models/taxon_observed.py ami/taxonomy/models/taxa_list.py etc.

And then move the views for taxonomy, and the management commands for loading / exporting taxa in that app as well.
ami/taxonomy/management/import_taxa.py

But I can understand doing this in stages as well.

ami/main/models_future/taxonomy/taxon.py
ami/main/views/api/taxonomy/taxon.py

What do you think?

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