Skip to content

Fix incorrect returned values when getting ascendants_and_descendants#498

Merged
mgeplf merged 1 commit intomainfrom
missing-ion-channel
Jan 6, 2026
Merged

Fix incorrect returned values when getting ascendants_and_descendants#498
mgeplf merged 1 commit intomainfrom
missing-ion-channel

Conversation

@mgeplf
Copy link
Copy Markdown
Collaborator

@mgeplf mgeplf commented Jan 6, 2026

  • When ascendants_and_descendants are chosen, and there are entities in the precise within_brain_region_brain_region_id, they would be retrieved twice - once from ascendants, and again from the descendants
  • pagination would limit the returned objects, but could include the duplicates, which were then removed by the unique in db.execute(data_query).scalars().unique()
  • this resulted in the number of objects returned being lower than what was requested in the page_size

* When `ascendants_and_descendants` are chosen, and there are entities
  in the precise `within_brain_region_brain_region_id`, they would be
  retrieved twice - once from `ascendants`, and again from the
  `descendants`
* pagination would limit the returned objects, but could include the
  duplicates, which were then removed by the `unique` in
  `db.execute(data_query).scalars().unique()`
@mgeplf
Copy link
Copy Markdown
Collaborator Author

mgeplf commented Jan 6, 2026

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
pytest 97.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
app/filters/brain_region.py 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@bilalesi bilalesi left a comment

Choose a reason for hiding this comment

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

I was skeptical about union_all, thanks @mgeplf

@mgeplf
Copy link
Copy Markdown
Collaborator Author

mgeplf commented Jan 6, 2026

This also should fix:
#474
As noted by @GianlucaFicarelli

@eleftherioszisis
Copy link
Copy Markdown
Contributor

This also should fix: #474 As noted by @GianlucaFicarelli

Would it be difficult to also include a small test that covers the #747 issue just to maintain our sanity with potential regressions in the future?

@mgeplf
Copy link
Copy Markdown
Collaborator Author

mgeplf commented Jan 6, 2026

Would it be difficult to also include a small test that covers the #747 issue just to maintain our sanity with potential regressions in the future?

That's what the page_size is doing here:
https://github.com/openbraininstitute/entitycore/pull/498/changes#diff-da4e1cbf774ac6257d994abd0e710ad69fadf968cc4e3480cd9a75055408ea49R389

Before the fix, it would have returned 1 entry, instead of 2.

Copy link
Copy Markdown
Contributor

@eleftherioszisis eleftherioszisis left a comment

Choose a reason for hiding this comment

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

lgtm

@mgeplf mgeplf merged commit b73cce9 into main Jan 6, 2026
2 checks passed
@mgeplf mgeplf deleted the missing-ion-channel branch January 6, 2026 09:41
GianlucaFicarelli added a commit that referenced this pull request Jan 16, 2026
* origin/main:
  Update `brain-atlas` endpoints (#506)
  Add published_in filter in ScientificArtifact (#505)
  Make nullable attributes of EMDenseReconstructionDataset (#503)
  Remove all endpoints that had a `InBrainRegionDep`, but don't have a brain_region_id (#501)
  Add pref_label/alt_label ilike_search to ETypeClass/MTypeClass filters (#500)
  add `number_neurons` to Simulation` (#493)
  Fix incorrect returned values when getting `ascendants_and_descendants` (#498)
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.

4 participants