Skip to content

Remove TaxonManager.with_children(). This method is never called, and… - #1507

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

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

Conversation

@kra

@kra kra commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

… the implentation is example code with a hardcoded key which will be an error if used.

Summary by CodeRabbit

  • Refactor
    • Removed the with_children() method for taxon queries.

@kra
kra added this pull request to stack #1489 October 9, 2026 17:31
@netlify

netlify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for antenna-ssec canceled.

Name Link
🔨 Latest commit 4c75062
🔍 Latest deploy log https://app.netlify.com/projects/antenna-ssec/deploys/6ac96f52e46b8000089e6ff6

@netlify

netlify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for antenna-preview canceled.

Name Link
🔨 Latest commit 4c75062
🔍 Latest deploy log https://app.netlify.com/projects/antenna-preview/deploys/6ac96f5291ff960008a131c6

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 29 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 24853cb8-1a73-40a1-94f2-6de1b7fe305e

📥 Commits

Reviewing files that changed from the base of the PR and between 17bce99 and 4c75062.


📒 Files selected for processing (1)
  • ami/main/models_future/taxonomy.py


No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1d14b256-9cb1-4491-a9d7-f5ac8211da78

📥 Commits

Reviewing files that changed from the base of the PR and between e4819a3 and 17bce99.


📒 Files selected for processing (1)
  • ami/main/models_future/taxonomy.py

💤 Files with no reviewable changes (1)
  • ami/main/models_future/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

The change removes TaxonManager.with_children(), which queried parents_json for child taxon IDs and used the query to annotate a queryset.

Changes

Taxonomy manager

Layer / File(s) Summary
Remove child annotation method
ami/main/models_future/taxonomy.py
TaxonManager.with_children() is removed. It queried parents_json for child taxon IDs and used the query to annotate a queryset.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to 17bce

This change removes an unused helper method from the taxonomy manager. No behavior change or production impact is expected, and no merge-blocking concerns were found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check Warning The description gives the reason for the removal but omits most required sections, including the summary, detailed description, testing information, deployment notes, and checklist. Complete the required pull request template. Add the summary, list of changes, related issues or state that none apply, detailed description with risks, testing steps and results, screenshots or an applicable statement, deployment notes, an…
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Title check Passed The title clearly identifies the removal of TaxonManager.with_children(), which matches the main change.

Full details: Description check

Resolution

Complete the required pull request template. Add the summary, list of changes, related issues or state that none apply, detailed description with risks, testing steps and results, screenshots or an applicable statement, deployment notes, and checklist status.



✨ Finishing Touches
📝 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.


logging.info(f"Updated parents for {len(bulk_update_data)} taxa")

def with_children(self):

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.

Funny I think this was a note to myself, because we do need to query all children for some missing features, and I will not remember how to do this:

        taxa = Taxon.objects.filter(parents_json__contains=[{"id": taxon.id}])
        qs = qs.annotate(children=models.Subquery(taxa.values("id")))

Maybe that can go in a docstring somewhere else.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

add it to the issue that adds the missing features?

… the implentation is example code with a hardoded key which will be an error if used.
@kra
kra force-pushed the kra/model-split-tests-storage-taxonomy-withchildren branch from 17bce99 to 4c75062 Compare October 9, 2026 22:48
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