search

scverse / scvi-tools

Deep probabilistic analysis of single-cell and spatial omics data
http://scvi-tools.org/
BSD 3-Clause "New" or "Revised" License
1.23k stars 346 forks source link

Manager summary stats should be per field #1744

Open adamgayoso opened 2 years ago

adamgayoso commented 2 years ago

Because of this line summary stats can be overwritten if two fields use the same name. Summary stats should be a dict with keys being the unique registry keys of the field and values being the summary stats

https://github.com/scverse/scvi-tools/blob/00e03420c5b7b59bd9ae57238bbfe431d9ab8865/scvi/data/_manager.py#L315

adamgayoso commented 2 years ago

what do you think @justjhong?

justjhong commented 2 years ago

It's a logical change, but also results in verbose access to the field-specific summary stats. If we could create an API to easily fetch summary stats (e.g. get_summary_stat(field name, stat key)) then it could work well enough. Mainly was being conservative about the structure of summary stats as a single high-level object when I implemented this.

martinkim0 commented 3 months ago

I think we should be raising an error here instead if someone has two fields with the same name.