Skip to content

Make the experimental design non-nullable (fix #1618) #1621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arteymix wants to merge 1 commit into
base: hotfix-1.32.7
Choose a base branch
from
Open

Make the experimental design non-nullable (fix #1618) #1621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gemma-cli/src/main/java/ubic/gemma/apps/DifferentialExpressionAnalysisCli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ protected void processExpressionExperiment( ExpressionExperiment ee ) {
return;
}

if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( ee + " does not have an experimental design populated." );
}

Expand Down
4 changes: 0 additions & 4 deletions gemma-cli/src/main/java/ubic/gemma/apps/SplitExperimentCli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ protected void processExpressionExperiment( ExpressionExperiment ee ) {
* Adapted from code in DifferentialExpressionAnalysisCli
*/
private ExperimentalFactor guessFactor( ExpressionExperiment ee ) {
if ( ee.getExperimentalDesign() == null ) {
throw new IllegalStateException( ee + " does not have an experimental design, it cannot be split on a factor." );
}

if ( this.factorName != null ) {

Collection<ExperimentalFactor> experimentalFactors = ee.getExperimentalDesign().getExperimentalFactors();
Expand Down
5 changes: 1 addition & 4 deletions gemma-cli/src/main/java/ubic/gemma/cli/util/EntityLocatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public ExperimentalFactor locateExperimentalFactor( ExpressionExperiment express

expressionExperiment = eeService.thawLiter( expressionExperiment );

if ( expressionExperiment.getExperimentalDesign() == null || expressionExperiment.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( expressionExperiment.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( "Experimental design is not populated for " + expressionExperiment + "." );
}

Expand Down Expand Up @@ -307,9 +307,6 @@ public ExperimentalFactor locateExperimentalFactor( ExpressionExperiment express

@Nullable
private ExperimentalFactor matchOneFactor( ExpressionExperiment ee, Predicate<ExperimentalFactor> predicate ) {
if ( ee.getExperimentalDesign() == null ) {
return null;
}
Set<ExperimentalFactor> matches = ee.getExperimentalDesign().getExperimentalFactors().stream()
.filter( predicate )
.collect( Collectors.toSet() );
Expand Down
16 changes: 3 additions & 13 deletions gemma-core/src/main/java/ubic/gemma/core/analysis/expression/diff/DiffExAnalyzerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,8 @@ public static AnalysisType determineAnalysisType( BioAssaySet bioAssaySet, Colle
}

private static Collection<ExperimentalFactor> getFactorsToUse( BioAssaySet bioAssaySet,
Collection<ExperimentalFactor> experimentalFactors ) {
Collection<ExperimentalFactor> efsToUse;

@Nullable Collection<ExperimentalFactor> experimentalFactors ) {
ExperimentalDesign design;

if ( bioAssaySet instanceof ExpressionExperiment ) {
design = ( ( ExpressionExperiment ) bioAssaySet ).getExperimentalDesign();
} else if ( bioAssaySet instanceof ExpressionExperimentSubSet ) {
Expand All @@ -306,10 +303,7 @@ private static Collection<ExperimentalFactor> getFactorsToUse( BioAssaySet bioAs
throw new UnsupportedOperationException( "Cannot deal with a " + bioAssaySet.getClass() );
}

if ( design == null ) {
throw new IllegalStateException( bioAssaySet + " does not have an experimental design." );
}

Collection<ExperimentalFactor> efsToUse;
if ( experimentalFactors == null || experimentalFactors.isEmpty() ) {
efsToUse = new HashSet<>( design.getExperimentalFactors() );
if ( efsToUse.isEmpty() ) {
Expand All @@ -318,18 +312,14 @@ private static Collection<ExperimentalFactor> getFactorsToUse( BioAssaySet bioAs
}
} else {
efsToUse = experimentalFactors;
if ( efsToUse.isEmpty() ) {
throw new IllegalArgumentException(
"No experimental factors. Cannot execute differential expression analysis." );
}

// sanity check...
for ( ExperimentalFactor experimentalFactor : efsToUse ) {
if ( !experimentalFactor.getExperimentalDesign().getId().equals( design.getId() ) ) {
throw new IllegalArgumentException( "Factors must come from the experiment provided" );
}
}
}

return efsToUse;
}

Expand Down
5 changes: 0 additions & 5 deletions ...java/ubic/gemma/core/analysis/preprocess/batcheffects/BatchInfoPopulationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,6 @@ static Map<String, String> readFastqHeaders( Path headerFile ) throws IOExceptio
private void removeExistingBatchFactor( ExpressionExperiment ee ) {
ExperimentalDesign ed = ee.getExperimentalDesign();

if ( ed == null ) {
log.warn( ee + " does not have an experimental design, cannot remove batch factor." );
return;
}

ExperimentalFactor toRemove = null;

for ( ExperimentalFactor ef : ed.getExperimentalFactors() ) {
Expand Down
6 changes: 0 additions & 6 deletions ...core/analysis/preprocess/batcheffects/ExpressionExperimentBatchCorrectionServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,6 @@ private ExpressionDataDoubleMatrix removeOutliers( ExpressionDataDoubleMatrix or
*/
@Nullable
private ExperimentalFactor getBatchFactor( ExpressionExperiment ee ) {
if ( ee.getExperimentalDesign() == null ) {
log.warn( ee + " does not have an experimental design, cannot get the batch factor." );
return null;
}
return ee.getExperimentalDesign().getExperimentalFactors().stream()
.filter( ExperimentFactorUtils::isBatchFactor )
.findFirst()
Expand Down Expand Up @@ -382,8 +378,6 @@ private ExpressionDataDoubleMatrix doComBat( ExpressionExperiment ee, Expression
*/
private ObjectMatrix<BioMaterial, ExperimentalFactor, Object> getDesign( ExpressionExperiment ee,
ExpressionDataDoubleMatrix mat ) {
Assert.notNull( ee.getExperimentalDesign(), ee + " does not have an experimental design." );

Collection<ExperimentalFactor> experimentalFactors = ee.getExperimentalDesign().getExperimentalFactors();

/* remove experimental factors that are for DE_Exclude */
Expand Down
19 changes: 5 additions & 14 deletions ...ore/analysis/preprocess/batcheffects/ExpressionExperimentBatchInformationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,6 @@ public BatchEffectDetails getBatchEffectDetails( ExpressionExperiment ee ) {
return details;
}

if ( ee.getExperimentalDesign() == null ) {
log.warn( ee + " have batch information, but it does not have an experimental design to determine the batch effect." );
return details;
}

ExperimentalFactor ef = ee.getExperimentalDesign().getExperimentalFactors()
.stream()
.filter( ExperimentFactorUtils::isBatchFactor )
Expand Down Expand Up @@ -307,15 +302,11 @@ private BatchInformationEvent checkBatchFetchStatus( ExpressionExperiment ee ) {
}

private boolean hasBatchFactor( ExpressionExperiment ee ) {
ee = expressionExperimentService.thawLiter( ee );
if ( ee.getExperimentalDesign() != null ) {
for ( ExperimentalFactor ef : ee.getExperimentalDesign().getExperimentalFactors() ) {
if ( ExperimentFactorUtils.isBatchFactor( ef ) ) {
return true;
}
}
}
return false;
return expressionExperimentService.thawLiter( ee )
.getExperimentalDesign()
.getExperimentalFactors()
.stream()
.anyMatch( ExperimentFactorUtils::isBatchFactor );
}

private boolean hasBeenBatchCorrected( ExpressionExperiment ee ) {
Expand Down
12 changes: 6 additions & 6 deletions gemma-core/src/main/java/ubic/gemma/core/analysis/service/ExpressionDataFileServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,8 @@ private int writeRawExpressionDataInternal( ExpressionExperiment ee, @Nullable L
@Override
public void writeDesignMatrix( ExpressionExperiment ee, boolean useProcessedData, Writer writer, boolean autoFlush ) throws IOException {
ee = expressionExperimentService.thawLite( ee );
if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( "No experimental design for " + ee );
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( "Empty experimental design for " + ee );
}
ExperimentalDesignWriter edWriter = new ExperimentalDesignWriter( entityUrlBuilder, buildInfo, autoFlush );
if ( useProcessedData ) {
Expand All @@ -742,8 +742,8 @@ public void writeDesignMatrix( ExpressionExperiment ee, boolean useProcessedData
@Override
public void writeDesignMatrix( ExpressionExperiment ee, QuantitationType qt, Class<? extends DataVector> vectorType, Writer writer, boolean autoFlush ) throws IOException {
ee = expressionExperimentService.thawLite( ee );
if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( "No experimental design for " + ee );
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
throw new IllegalStateException( "Empty experimental design for " + ee );
}
ExperimentalDesignWriter edWriter = new ExperimentalDesignWriter( entityUrlBuilder, buildInfo, autoFlush );
Collection<BioAssayDimension> processedDimension = expressionExperimentService.getBioAssayDimensionsWithAssays( ee, qt );
Expand Down Expand Up @@ -907,7 +907,7 @@ public LockedPath writeOrLocateRawExpressionDataFile( ExpressionExperiment ee, Q
@Override
public Optional<LockedPath> writeOrLocateDesignFile( ExpressionExperiment ee, boolean useProcessedData, boolean forceWrite ) throws IOException {
ee = expressionExperimentService.thawLite( ee );
if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
return Optional.empty();
}
try ( LockedPath f = this.getOutputFile( getDesignFileName( ee, useProcessedData ), false ) ) {
Expand All @@ -929,7 +929,7 @@ public Optional<LockedPath> writeOrLocateDesignFile( ExpressionExperiment ee, bo
@Override
public Optional<LockedPath> writeOrLocateDesignFile( ExpressionExperiment ee, boolean useProcessedData, boolean forceWrite, long timeout, TimeUnit timeUnit ) throws TimeoutException, IOException, InterruptedException {
ee = expressionExperimentService.thawLite( ee );
if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
return Optional.empty();
}
try ( LockedPath f = this.getOutputFile( getDesignFileName( ee, useProcessedData ), false, timeout, timeUnit ) ) {
Expand Down
4 changes: 2 additions & 2 deletions ...-core/src/main/java/ubic/gemma/core/datastructure/matrix/io/ExperimentalDesignWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ public void write( ExpressionExperiment ee, Writer writer ) throws IOException {
* for details
*/
public void write( ExpressionExperiment ee, @Nullable QuantitationType quantitationType, @Nullable Class<? extends DataVector> vectorType, Collection<BioAssay> bioAssays, boolean writeBaseHeader, Writer writer ) throws IOException {
Assert.isTrue( ee.getExperimentalDesign() != null && !ee.getExperimentalDesign().getExperimentalFactors().isEmpty(),
ee + " does not have an experimental design." );
Assert.isTrue( !ee.getExperimentalDesign().getExperimentalFactors().isEmpty(),
ee + " has an empty experimental design." );

ExperimentalDesign ed = ee.getExperimentalDesign();

Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/ubic/gemma/core/visualization/ExperimentalDesignVisualizationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private LinkedHashMap<BioAssayValueObject, LinkedHashMap<ExperimentalFactor, Dou

Map<FactorValue, Double> fvV = new HashMap<>();

if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
// Case of no experimental design; just put in a dummy factor.
ExperimentalFactor dummyFactor = ExperimentalFactor.Factory.newInstance( "No factors", FactorType.CATEGORICAL );
for ( BioMaterial bm : bms ) {
Expand Down
14 changes: 4 additions & 10 deletions ...re/src/main/java/ubic/gemma/core/visualization/cellbrowser/CellBrowserMetadataWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,10 @@ public void write( ExpressionExperiment ee, SingleCellDimension singleCellDimens
.flatMap( Collection::stream )
.map( FactorValue::getExperimentalFactor )
.collect( Collectors.toSet() );
List<ExperimentalFactor> factors;
if ( ee.getExperimentalDesign() != null ) {
factors = ee.getExperimentalDesign().getExperimentalFactors().stream()
.filter( usedFactors::contains )
.sorted( ExperimentalFactor.COMPARATOR )
.collect( Collectors.toList() );
} else {
log.warn( ee + " does not have an experimental design, no factors will be written." );
factors = Collections.emptyList();
}
List<ExperimentalFactor> factors = ee.getExperimentalDesign().getExperimentalFactors().stream()
.filter( usedFactors::contains )
.sorted( ExperimentalFactor.COMPARATOR )
.collect( Collectors.toList() );
Map<ExperimentalFactor, Map<BioMaterial, FactorValue>> factorValueMap = ExperimentalDesignUtils.getFactorValueMap( factors, samples );
SortedMap<Category, Map<BioAssay, Characteristic>> bioAssayCharacteristics = selectCharacteristics( BioAssayUtils.createCharacteristicMap( assays ), assays.size() );
SortedMap<Category, Map<BioMaterial, Characteristic>> sampleCharacteristics = selectCharacteristics( createCharacteristicMap( samples ), samples.size() );
Expand Down
4 changes: 1 addition & 3 deletions gemma-core/src/main/java/ubic/gemma/model/expression/experiment/ExpressionExperiment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public static ExpressionExperiment newInstance() {
@Nullable
private String batchConfound;
private CurationDetails curationDetails = new CurationDetails();
@Nullable
private ExperimentalDesign experimentalDesign;
@Nullable
private Geeq geeq;
Expand Down Expand Up @@ -190,7 +189,6 @@ public CurationDetails getCurationDetails() {
return this.curationDetails;
}

@Nullable
@IndexedEmbedded
public ExperimentalDesign getExperimentalDesign() {
return this.experimentalDesign;
Expand Down Expand Up @@ -291,7 +289,7 @@ public void setCurationDetails( CurationDetails curationDetails ) {
this.curationDetails = curationDetails;
}

public void setExperimentalDesign( @Nullable ExperimentalDesign experimentalDesign ) {
public void setExperimentalDesign( ExperimentalDesign experimentalDesign ) {
this.experimentalDesign = experimentalDesign;
}

Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/ubic/gemma/model/expression/experiment/ExpressionExperimentValueObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public ExpressionExperimentValueObject( ExpressionExperiment ee, boolean ignoreD
}

// ED
if ( !ignoreDesign && ee.getExperimentalDesign() != null && ModelUtils.isInitialized( ee.getExperimentalDesign() ) ) {
if ( !ignoreDesign && ModelUtils.isInitialized( ee.getExperimentalDesign() ) ) {
this.experimentalDesign = ee.getExperimentalDesign().getId();
}

Expand Down
15 changes: 4 additions & 11 deletions gemma-core/src/main/java/ubic/gemma/persistence/persister/ExpressionPersister.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,10 @@ protected ExpressionExperiment persistExpressionExperiment( ExpressionExperiment

// This has to come first and be persisted, so our FactorValues get persisted before we process the
// BioAssays.
if ( ee.getExperimentalDesign() != null ) {
ExperimentalDesign experimentalDesign = ee.getExperimentalDesign();
this.processExperimentalDesign( experimentalDesign, caches );
assert experimentalDesign.getId() != null;
ee.setExperimentalDesign( experimentalDesign );
}
ExperimentalDesign experimentalDesign = ee.getExperimentalDesign();
this.processExperimentalDesign( experimentalDesign, caches );
assert experimentalDesign.getId() != null;
ee.setExperimentalDesign( experimentalDesign );

this.checkExperimentalDesign( ee );

Expand Down Expand Up @@ -161,11 +159,6 @@ protected <T extends Identifiable> T doPersist( T entity, Caches caches ) {
* If there are factorValues, check if they are setup right and if they are used by biomaterials.
*/
private void checkExperimentalDesign( ExpressionExperiment expExp ) {
if ( expExp.getExperimentalDesign() == null ) {
AbstractPersister.log.warn( "No experimental design!" );
return;
}

Collection<ExperimentalFactor> efs = expExp.getExperimentalDesign().getExperimentalFactors();

if ( efs.isEmpty() )
Expand Down
2 changes: 1 addition & 1 deletion ...tence/service/expression/bioAssayData/ProcessedExpressionDataVectorHelperServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class ProcessedExpressionDataVectorHelperServiceImpl
@Override
@Transactional
public void reorderByDesign( ExpressionExperiment ee ) {
if ( ee.getExperimentalDesign() == null || ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
if ( ee.getExperimentalDesign().getExperimentalFactors().isEmpty() ) {
log.info( ee + " does not have a populated experimental design, skipping" );
return;
}
Expand Down
10 changes: 4 additions & 6 deletions ...ava/ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2361,13 +2361,11 @@ public void thawLiter( final ExpressionExperiment expressionExperiment ) {

Hibernate.initialize( expressionExperiment.getOtherParts() );

if ( expressionExperiment.getExperimentalDesign() != null ) {
for ( ExperimentalFactor ef : expressionExperiment.getExperimentalDesign().getExperimentalFactors() ) {
Hibernate.initialize( ef );
ef.getFactorValues().forEach( Hibernate::initialize );
}
Hibernate.initialize( expressionExperiment.getExperimentalDesign().getTypes() );
for ( ExperimentalFactor ef : expressionExperiment.getExperimentalDesign().getExperimentalFactors() ) {
Hibernate.initialize( ef );
ef.getFactorValues().forEach( Hibernate::initialize );
}
Hibernate.initialize( expressionExperiment.getExperimentalDesign().getTypes() );

if ( expressionExperiment.getPrimaryPublication() != null ) {
thawBibliographicReference( expressionExperiment.getPrimaryPublication() );
Expand Down
12 changes: 0 additions & 12 deletions ...ubic/gemma/persistence/service/expression/experiment/ExpressionExperimentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,6 @@ public ExperimentalFactor addFactor( ExpressionExperiment ee, ExperimentalFactor
factor.setExperimentalDesign( experiment.getExperimentalDesign() );
factor.setSecurityOwner( experiment );
factor = experimentalFactorService.create( factor ); // to make sure we get acls.
if ( experiment.getExperimentalDesign() == null ) {
log.info( "Creating missing experimental design for " + experiment );
experiment.setExperimentalDesign( new ExperimentalDesign() );
}
experiment.getExperimentalDesign().getExperimentalFactors().add( factor );
expressionExperimentDao.update( experiment );
return factor;
Expand All @@ -245,10 +241,6 @@ public FactorValue addFactorValue( ExpressionExperiment ee, FactorValue fv ) {
assert fv.getExperimentalFactor() != null;
ExpressionExperiment experiment = requireNonNull( expressionExperimentDao.load( ee.getId() ) );
fv.setSecurityOwner( experiment );
if ( experiment.getExperimentalDesign() == null ) {
log.info( "Creating missing experimental design for " + experiment );
experiment.setExperimentalDesign( new ExperimentalDesign() );
}
Collection<ExperimentalFactor> efs = experiment.getExperimentalDesign().getExperimentalFactors();
fv = this.factorValueService.create( fv );
for ( ExperimentalFactor ef : efs ) {
Expand All @@ -266,10 +258,6 @@ public FactorValue addFactorValue( ExpressionExperiment ee, FactorValue fv ) {
@Transactional
public void addFactorValues( ExpressionExperiment ee, Map<BioMaterial, FactorValue> fvs ) {
ExpressionExperiment experiment = requireNonNull( expressionExperimentDao.load( ee.getId() ) );
if ( experiment.getExperimentalDesign() == null ) {
log.info( "Creating missing experimental design for " + experiment );
experiment.setExperimentalDesign( new ExperimentalDesign() );
}
Collection<ExperimentalFactor> efs = experiment.getExperimentalDesign().getExperimentalFactors();
int count = 0;
for ( BioMaterial bm : fvs.keySet() ) {
Expand Down
Loading