Jim Procter

one class with a static factory method seems to be better than two classes and an external mutable object.

one class with a static factory method seems to be better than two classes and an external mutable object.

yes, I see that, but do you really need a nested static class ? in line with my general comments re encapsulating the Map<String,SequenceInfo> further: perhaps it might make sense to simply apply a...

yes, I see that, but do you really need a nested static class ? in line with my general comments re encapsulating the Map<String,SequenceInfo> further: perhaps it might make sense to simply apply a factory pattern rather than have an internal bean that a generic map holds references to ?

(lets not get stuck on this though, the main thing is that the implementation works fine for now, though needs more work in order to save/restore sequenceInfo objects from Jalview project files)

we won't see this messages unless asserts are enabled. Cache.log.warn is perhaps better ?

we won't see this messages unless asserts are enabled. Cache.log.warn is perhaps better ?

it's a reasonable first pass, though stylistically all you've done is transformed a single concrete instance (Hashtable) to a more strongly typed verbose instance (Map<String,SequenceInfo>). Why no...

it's a reasonable first pass, though stylistically all you've done is transformed a single concrete instance (Hashtable) to a more strongly typed verbose instance (Map<String,SequenceInfo>). Why not go further and have a SequencesInfo object that records metadata for one or more sequences ?

There are also other requirements: SequenceInfo sets will need to be persisted for a web services job. You could expand JAL-3899 to incorporate this perhaps - JAL-1786 is the relevant epic for that.

why does this class need to be static ?

why does this class need to be static ?

this should probably be Cache.log.warn(..)

this should probably be Cache.log.warn(..)

JAL-3899 Refactor sequence de/uniquification.
JAL-3899 Refactor sequence de/uniquification.
after taking a look at the rest I can see that the next step is to split this class up further. There's plenty of code that will be used by many different operations - e.g. job state management - i...

after taking a look at the rest I can see that the next step is to split this class up further. There's plenty of code that will be used by many different operations - e.g. job state management - is that the plan ?

not really the recommended way - see e.g. https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/java/beans/PropertyChangeSupport.html *do you need to expose the generic binders ? what k...

not really the recommended way - see e.g. https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/java/beans/PropertyChangeSupport.html

  • do you need to expose the generic binders ? what kind of events need to be monitored ?
is utils really the right name ? suggest the classes are used in several different places

is utils really the right name ? suggest the classes are used in several different places

this is going to get much more complex. there are plenty of additional attributes - e.g. input type (dna, protein, rna) - none of them are currently being configured on the operation.

this is going to get much more complex. there are plenty of additional attributes - e.g. input type (dna, protein, rna) - none of them are currently being configured on the operation.

this class seems to contain a bunch of hard coded stuff taken from the original web services configuration code rather than provide a way for a web services provider to configure an instance of Ali...

this class seems to contain a bunch of hard coded stuff taken from the original web services configuration code rather than provide a way for a web services provider to configure an instance of AlignmentOperation tuned to the alignment service that the provider is offering.

first review of Web Services overhaul for 2.12
first review of Web Services overhaul for 2.12
patch needed for 2.11.2 - JAL-3518 - catch case when no replies are available

patch needed for 2.11.2 - JAL-3518 - catch case when no replies are available

semantics for determining if a PDB file has a SIFTs mapping - could be absorbed into the PDB metadata object

semantics for determining if a PDB file has a SIFTs mapping - could be absorbed into the PDB metadata object

magic logic to recognise which structure database provider client to use - PDB or AlphaFold - ensures metadata and annotation rows are propagates

magic logic to recognise which structure database provider client to use - PDB or AlphaFold - ensures metadata and annotation rows are propagates

Reviewing the alphafold + contact matrix patch for 2.11.2
Reviewing the alphafold + contact matrix patch for 2.11.2
remove debugging printlns

remove debugging printlns

aim is normally to avoid creating query-specific data containers on the core query response class

aim is normally to avoid creating query-specific data containers on the core query response class

? not sure this is correct

? not sure this is correct

Needs additionally: model_category created sequence_identity uniprot_start uniprot_end resolution coverage qmean_version ? model_version

Needs additionally:
model_category
created
sequence_identity
uniprot_start
uniprot_end
resolution
coverage
qmean_version ?
model_version

Evaluate 3D-Beacons dev path based on Arnaud's ERASMUS+ project code
Evaluate 3D-Beacons dev path based on Arnaud's ERASMUS+ project code