Feat multispec#251
Conversation
| * @return The {@code Stream} of {@code SimilarityExpressionCall}s that are | ||
| * the similar expression calls for the requested parameters. | ||
| */ | ||
| public Stream<SimilarityExpressionCall> loadSimilarityExpressionCalls(int taxonId, |
There was a problem hiding this comment.
I think we discussed this method should work as for org.bgee.model.expressiondata.call.ExpressionCallLoader and org.bgee.model.expressiondata.call.ExpressionCallService?
Meaning, this method should accept a org.bgee.model.expressiondata.call.CallFilter.ExpressionCallFilter2, or a new class extending org.bgee.model.expressiondata.call.CallFilter to accept multiple GeneFilters?
Then under the hood, the code could use the ExpressionCallLoader or ExpressionCallService directly
| Set<AnatEntitySimilarity> anatEntitySimilaritiesFromCellTypeFilter = | ||
| new HashSet<>(anatEntitySimilaritiesFromAnatFilter); | ||
|
|
||
| // Extract anat entity and cell type IDs from conditionFilters for filtering similarities |
There was a problem hiding this comment.
This logic should go in a separate function probably, returning the Map<ConditionParameter<?, ?>, ComposedFilterIds<String>> condParamToFilter
There was a problem hiding this comment.
Also, don't separate anatEntities and CellTypes work, do everything at once, and separate at the end using methd isCellType() for line 1107
| } | ||
| filterAnatEntityIds.addAll(expandedAnatIds); | ||
| } | ||
| if (cellIds != null) { |
There was a problem hiding this comment.
Refactor to avoid code duplication (same as for anatIds)
Probably it's more optimal to instantiate the ontology once for both anat and cell type at the same time
|
|
||
| if (!filterAnatEntityIds.isEmpty()) { | ||
| anatEntitySimilaritiesFromAnatFilter = anatEntitySimilaritiesFromAnatFilter.stream() | ||
| .filter(s -> s.getSourceAnatEntities().stream() |
There was a problem hiding this comment.
Shouldn't it use getAllAnatEntities?
| // Build maps for grouping (same logic as loadSimilarityExpressionCalls) | ||
| Map<AnatEntity, Set<AnatEntitySimilarity>> similaritiesByAnatEntityFromAnatFilter = | ||
| anatEntitySimilaritiesFromAnatFilter.stream() | ||
| .flatMap(sim -> sim.getSourceAnatEntities().stream() |
| .collect(Collectors.toList()); | ||
|
|
||
| if (this.requestParameters.isGetResultCount()) { | ||
| count = (long) allCalls.size(); |
There was a problem hiding this comment.
Yeah, so here it shows that the code hasn't be written as the ExpressionCallLoader, and all results are always all in memory. Is that OK?
| if (offset < 0) { | ||
| throw log.throwing(new InvalidRequestException("Offset cannot be less than 0.")); | ||
| } | ||
| calls = allCalls.stream() |
| .collect(Collectors.toList()); | ||
| } | ||
| if (this.requestParameters.isGetFilters()) { | ||
| // Multi-species endpoint does not support post-filters; ignore. |
There was a problem hiding this comment.
throw InvalidRequestException then
| log.traceExit(); | ||
| } | ||
|
|
||
| private MultispecExprCallItem toMultispecExprCallItem(SimilarityExpressionCall2 sc) { |
There was a problem hiding this comment.
Feel lazy to read, explain to me why it's needed when we meet :p
| <groupId>org.bgee.log4jdbc-log4j2</groupId> | ||
| <artifactId>log4jdbc-log4j2-jdbc4.1</artifactId> | ||
| <version>1.17-SNAPSHOT</version> | ||
| <version>1.16</version> |
There was a problem hiding this comment.
Check whether this should be merge back
| * @return The {@code Stream} of {@code SimilarityExpressionCall2}s that are | ||
| * the similar expression calls for the requested parameters. | ||
| */ | ||
| public Stream<SimilarityExpressionCall2> loadSimilarityExpressionCalls2(int taxonId, |
There was a problem hiding this comment.
You can extend CallFilter to create a MultiSpeciesExpressionCallFilter accepting multiple GeneFilters, taxonId, and the boolean onlyTrusted
| throw log.throwing(new IllegalArgumentException("No gene filter should be null")); | ||
| } | ||
|
|
||
| Set<GeneFilter> clnGeneFilters = Collections.unmodifiableSet( |
There was a problem hiding this comment.
Set clnGeneFilters = geneFilters == null || geneFilters.isEmpty() ?Collections.unmodifiableSet(this.getServiceFactory().getSpeciesService()
.loadSpeciesByTaxonIds(Collections.singleton(taxonId), false)
.stream().map(s -> new GeneFilter(s.getId()))
.collect(Collectors.toSet())) :
geneFilters;
| Set<AnatEntitySimilarity> anatEntitySimilaritiesFromCellTypeFilter = | ||
| new HashSet<>(anatEntitySimilaritiesFromAnatFilter); | ||
|
|
||
| // Extract anat entity and cell type IDs from conditionFilters for filtering similarities |
There was a problem hiding this comment.
Also, don't separate anatEntities and CellTypes work, do everything at once, and separate at the end using methd isCellType() for line 1107
|
|
||
| // Load ExpressionCall2 for each species and merge (ExpressionCallFilter2 has single GeneFilter) | ||
| List<ExpressionCall2> allCalls = new ArrayList<>(); | ||
| for (GeneFilter gf : clnGeneFilters) { |
There was a problem hiding this comment.
Maybe a solution not to put everything in memory at once for a given GeneFilter, and still retrieving all data at once for `AnatEntitySimilarity, would be to have 2 interleaved loops:
for (AnatEntitySimilarity sim: sims) {
for (GeneFilter gf : clnGeneFilters) {
call ExpressionCallLoader
for (Gene gene in returned ExpressionCalls) {
=> produce ONE SimilarityExpressionCall
}
}
}
Should we put all orthologous genes in a SimilarityExpressionCall?
Should we provide "pagination" on number of AnatEntitySimilaritys iterated? Or on number of SimilarityExpressionCalls returned? But then, complicated to know which AnatEntitySimilarity and GeneFilter to restart the procedure from
| * Extracts the anatomical entity from a {@code Condition2} for ANAT_ENTITY_CELL_TYPE. | ||
| * ComposedEntity order: cellType at index 0, anatEntity at index 1 when both present. | ||
| */ | ||
| private static AnatEntity getAnatEntityFromCondition2( |
There was a problem hiding this comment.
See contructor of Condition2
1 participant
PR solely for being able to comment in diff