Activity jalview/

3:27 pm

Jim Procter <j.procter@dundee.ac.uk>

Merge branch 'bug/r2_11_2/JAL-3904_structureviewermenu' into develop

3:04 pm

Jim Procter <j.procter@dundee.ac.uk>

More
05e40b4
changed 2 files
bug/r2_11_2/JAL-3904_structureviewermenu, develop, releases/Release_2_11_2_Branch

JAL-3905 Fix: wait for response when sending mouseovers. Combined remove old label and add new label and also added warning in javadoc about out-of-order execution

- -7
- +24
/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
- History
- Source
- Diff
/src/jalview/.../JalviewChimeraBinding.java
- -1
- +4
/src/jalview/structures/models/AAStructureBindingModel.java
- History
- Source
- Diff
/src/jalview/.../AAStructureBindingModel.java

1:18 pm

Jim Procter <j.procter@dundee.ac.uk>

More
8f08ed7
changed 3 files
bug/r2_11_2/JAL-3904_structureviewermenu, develop, releases/Release_2_11_2_Branch

JAL-3904 override buildActionMenu to add in additional menu actions after rebuild

- -4
- +15
/src/jalview/gui/ChimeraViewFrame.java
- History
- Source
- Diff
/src/jalview/gui/ChimeraViewFrame.java
- -2
- +9
/src/jalview/gui/PymolViewer.java
- History
- Source
- Diff
/src/jalview/gui/PymolViewer.java
- -1
- +7
/src/jalview/gui/StructureViewerBase.java
- History
- Source
- Diff
/src/jalview/gui/StructureViewerBase.java

12:39 pm

Jim Procter <j.procter@dundee.ac.uk>

More
f2c4dcb
changed 3 files
bug/r2_11_2/JAL-3904_structureviewermenu, develop, releases/Release_2_11_2_Branch

JAL-3904 failing test

- -0
- +6
/src/jalview/api/structures/JalviewStructureDisplayI.java
- History
- Source
- Diff
/src/.../JalviewStructureDisplayI.java
- -1
- +5
/src/jalview/gui/StructureViewerBase.java
- History
- Source
- Diff
/src/jalview/gui/StructureViewerBase.java
- -2
- +30
/test/jalview/ext/rbvi/chimera/JalviewChimeraView.java
- History
- Source
- Diff
/test/jalview/.../JalviewChimeraView.java

12:53 pm

Mateusz Warowny

More
Re: CR-JAL-254
commented on review

SequencesInfo which is an individual object is doable, but I feel like it would be just a wrapper around a Map. What additional functionality would SequencesInfo have over a traditional Map? I'm n...

SequencesInfo which is an individual object is doable, but I feel like it would be just a wrapper around a Map. What additional functionality would SequencesInfo have over a traditional Map?

I'm not very familiar with serialization in java, I know there are libraries that can serialize beans to json or xml, but what are your expectations in this case?

11:57 am

Jim Procter

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

11:34 am

Jim Procter

More
CR-JAL-254
resumed reviewing

11:31 am

Mateusz Warowny

More
Re: CR-JAL-254
replied to Jim Procter

What's wrong with static classes? They are just like regular classes but within other class' namespace. That's non-static inner classes that are special. About factory pattern, I think it would be ...

What's wrong with static classes? They are just like regular classes but within other class' namespace. That's non-static inner classes that are special.
About factory pattern, I think it would be an overkill for a class that is only used by SeqsetUtils' methods.

11:16 am

Jim Procter

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)

10:58 am

Mateusz Warowny

More
Re: CR-JAL-254
replied to Jim Procter

Because it's instantiated in a static context e.g. uniquify and it does not need an instance of SeqsetUtils object to work.

8:56 am

Jim Procter

More
Re: CR-JAL-254
commented on review

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

8:53 am

Jim Procter

More
Re: CR-JAL-254
commented on review

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.

8:42 am