✨ Add `.artifacts_load()`, `.artifacts_open()`, `.artifacts_mapped()` to `QuerySet` #2546

Koncopd · 2025-03-10T15:16:12Z

Adds QuerySet.artifacts_load(), QuerySet.artifacts_open(), QuerySet.artifacts_mapped() that do the same as Collection.load(), Collection.open(), Collection.mapped() for query sets of artifacts. This allows to avoid creating collections if only some loading or opening is needed.

Use:

artifacts = ln.Artifact.filter(..., otype="AnnData").order_by("created_at") # order_by to avoid arbitrary order warning
artifacts.artifacts_open()
artifacts.artifacts_load()
artifacts.artifacts_mapped()

sunnyosun · 2025-03-10T15:18:36Z

Why not just QuerySet.open() and QuerySet.mapped()? I think that's more in parallel with the current APIs. Maybe raising an error if the QuerySet is not artifacts?

Koncopd · 2025-03-10T15:20:10Z

Maybe raising an error if the QuerySet is not artifacts?

Sure.

Why not just QuerySet.open() and QuerySet.mapped()? I think that's more in parallel with the current APIs.

Hm, i wanted to emphasize that it works only for query sets of artifacts. You don't like it?

github-actions · 2025-03-10T15:28:58Z

🚀 Deployed on https://67d448e670d5042755bd7825--lamindb-qnwk.netlify.app

codecov · 2025-03-10T15:49:17Z

Codecov Report

Attention: Patch coverage is 95.12195% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.06%. Comparing base (22f4071) to head (bc11bd9).
Report is 114 commits behind head on main.

Files with missing lines	Patch %	Lines
lamindb/models/collection.py	94.44%	2 Missing ⚠️
lamindb/models/query_set.py	95.65%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2546      +/-   ##
==========================================
- Coverage   92.42%   92.06%   -0.36%     
==========================================
  Files          60       58       -2     
  Lines        9989     8638    -1351     
==========================================
- Hits         9232     7953    -1279     
+ Misses        757      685      -72

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Koncopd · 2025-03-10T17:25:08Z

Also a thing to discuss. Now i raise an error if the model of a query set is not Artifact. Maybe i should just query related artifacts instead in this case?

github-actions · 2025-03-18T11:06:32Z

Deployment URL: https://dbb6b8d1.lamindb.pages.dev

sunnyosun · 2025-03-18T12:29:27Z

Also a thing to discuss. Now i raise an error if the model of a query set is not Artifact. Maybe i should just query related artifacts instead in this case?

I think the error is good! Querying the related would be too implicit in this case.

falexwolf

I think we need a class ArtifactQuerySet if we want to do this.

We cannot bloat the generic QuerySet API with methods that won't make sense for almost all query sets, can we?

sunnyosun · 2025-03-18T12:30:39Z

docs/arrays.ipynb

I still think backed = ln.Artifact.filter(suffix=".parquet").open() would be a lot prettier here 😊

sunnyosun · 2025-03-18T12:33:00Z

lamindb/models/collection.py

+    # Why is that? - Sergei
+    if len(suffixes) != 1:
+        raise ValueError(
+            "Can only load collections where all artifacts have the same suffix"


The error needs to also adapt to different functions, right? Now it's not only loading collections, can also be Queryset

sunnyosun · 2025-03-18T12:38:25Z

lamindb/models/query_set.py

+
+        Returns an in-memory concatenated `DataFrame` or `AnnData` object.
+
+        See also {meth}`~lamindb.models.Collecton.load`.


update the ref link

Could you also add Example:: to the docstring?

sunnyosun · 2025-03-18T12:48:35Z

lamindb/models/collection.py

+
+
+def _load_concat_artifacts(
+    artifacts: list[Artifact], join: Literal["inner", "outer"] = "outer", **kwargs


**kwargs → **concat_kwargs?

Please no **kwargs at all!

falexwolf · 2025-03-18T12:56:41Z

Quite generally the name choice artifacts_load() etc. is very confusing and I'd not guess what it does.

If we have ArtifactQuerySet.load() that would be more understandable.

falexwolf · 2025-03-18T13:05:12Z

Also: we have to find a way of getting rid of monkey-patching Django's QuerySet. I can't think of a simple solution so far but it's pretty annoying that we have this in our codebase (same for QueryManager).

Koncopd · 2025-03-18T13:30:37Z

I think we need a class ArtifactQuerySet if we want to do this.

Yes, but it is much harder to implement as we have to account for cases where django QuerySet is returned. Now we use monkey patching for this, but it is impossible to do for something like ArtifactQuerySet. I will think on what to do, i have some ideas for how to get rid of monkey patching here.

falexwolf · 2025-03-18T13:41:20Z

Yes, but it is much harder to implement as we have to account for cases where django QuerySet is returned. Now we use monkey patching for this, but it is impossible to do for something like ArtifactQuerySet.

I know it's much harder. But we have to invest the effort now. No more new hacks. They'll all just bite us. We gotta keep focusin on removing the hacks of the early days to improve quality and performance.

I will think on what to do, i have some ideas for how to get rid of monkey patching here.

That'd be great!

Koncopd · 2025-04-29T15:53:44Z

To do this properly, i need something like this #2637 first.

falexwolf · 2025-04-29T16:00:05Z

Makes total sense!

Koncopd · 2025-05-05T11:49:02Z

Continued here #2743

github-actions bot temporarily deployed to pull request March 10, 2025 15:28 Inactive

Koncopd marked this pull request as draft March 10, 2025 15:41

github-actions bot temporarily deployed to pull request March 10, 2025 16:21 Inactive

github-actions bot temporarily deployed to pull request March 10, 2025 20:41 Inactive

Koncopd force-pushed the queryset_open_mapped branch 2 times, most recently from b2173b7 to 41e056b Compare March 14, 2025 12:48

github-actions bot temporarily deployed to pull request March 14, 2025 12:59 Inactive

github-actions bot temporarily deployed to pull request March 14, 2025 14:50 Inactive

Koncopd changed the title ~~✨ Add QuerySet.artifacts_open() and QuerySet.artifacts_mapped()~~ ✨ Add .artifacts_load(), .artifacts_open(), .artifacts_mapped() to QuerySet Mar 14, 2025

Koncopd force-pushed the queryset_open_mapped branch from 0a98baa to 6ceaf0c Compare March 14, 2025 15:08

github-actions bot temporarily deployed to pull request March 14, 2025 15:19 Inactive

Koncopd added 14 commits March 18, 2025 11:52

artifacts_open

2198e1b

type

7d3bb4c

artifacts_mapped

57d7186

reorder

db2d3b5

test mapped

6bea09c

fix and test

be933aa

fix

61157c7

rename the test

55c2e15

test artifacts_open

250ae98

test errors

7076e3b

docstr

5d7266b

refactor collection.load

955afe4

Add artifacts_load

958dda2

some more testing

06b53cf

Koncopd force-pushed the queryset_open_mapped branch from 6ceaf0c to 06b53cf Compare March 18, 2025 10:54

github-actions bot deployed to preview March 18, 2025 11:06 View deployment

Koncopd added 2 commits March 18, 2025 12:41

test artifacts_load

6a817a5

fix

d549969

github-actions bot deployed to preview March 18, 2025 12:03 View deployment

some more testing

bc11bd9

github-actions bot deployed to preview March 18, 2025 12:15 View deployment

Koncopd marked this pull request as ready for review March 18, 2025 12:26

Koncopd requested review from falexwolf and sunnyosun March 18, 2025 12:26

falexwolf requested changes Mar 18, 2025

View reviewed changes

sunnyosun reviewed Mar 18, 2025

View reviewed changes

falexwolf force-pushed the main branch from f6030cd to af28cf8 Compare March 26, 2025 16:27

Koncopd closed this May 5, 2025

falexwolf deleted the queryset_open_mapped branch October 15, 2025 20:31


		Returns an in-memory concatenated `DataFrame` or `AnnData` object.

		See also {meth}`~lamindb.models.Collecton.load`.



		def _load_concat_artifacts(
		artifacts: list[Artifact], join: Literal["inner", "outer"] = "outer", **kwargs

✨ Add .artifacts_load(), .artifacts_open(), .artifacts_mapped() to QuerySet #2546

✨ Add .artifacts_load(), .artifacts_open(), .artifacts_mapped() to QuerySet #2546

Uh oh!

Conversation

Koncopd commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunnyosun commented Mar 10, 2025

Uh oh!

Koncopd commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Koncopd commented Mar 10, 2025

Uh oh!

github-actions bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunnyosun commented Mar 18, 2025

Uh oh!

falexwolf left a comment

Choose a reason for hiding this comment

Uh oh!

sunnyosun Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

sunnyosun Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

sunnyosun Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

sunnyosun Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

sunnyosun Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

falexwolf Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

falexwolf commented Mar 18, 2025

Uh oh!

falexwolf commented Mar 18, 2025

Uh oh!

Koncopd commented Mar 18, 2025

Uh oh!

falexwolf commented Mar 18, 2025

Uh oh!

Koncopd commented Apr 29, 2025

Uh oh!

falexwolf commented Apr 29, 2025

Uh oh!

Koncopd commented May 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

✨ Add `.artifacts_load()`, `.artifacts_open()`, `.artifacts_mapped()` to `QuerySet` #2546

✨ Add `.artifacts_load()`, `.artifacts_open()`, `.artifacts_mapped()` to `QuerySet` #2546

Koncopd commented Mar 10, 2025 •

edited

Loading

Koncopd commented Mar 10, 2025 •

edited

Loading

github-actions bot commented Mar 10, 2025 •

edited

Loading

codecov bot commented Mar 10, 2025 •

edited

Loading

github-actions bot commented Mar 18, 2025 •

edited

Loading