Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
JAL-3210 Barebones gradle/buildship/eclipse. See README

  1. … 1990 more files in changeset.
JAL-2944: Allow structures to be opened in a specific structure view
JAL-2944: Allow structures to be opened in a specific structure view
Updated with latest from mchmmer branch

  1. … 221 more files in changeset.
JAL-2674 Transferred updated unit tests from first 2674 branch
JAL-2674 Transferred updated unit tests from first 2674 branch
Merge branch 'develop' into feature/JAL-2759

Conflicts:

benchmarking/README

src/jalview/datamodel/Alignment.java

src/jalview/datamodel/AlignmentAnnotation.java

src/jalview/gui/SeqCanvas.java

src/jalview/gui/SeqPanel.java

  1. … 7 more files in changeset.
ok

ok

ok

ok

can replace method body with this(null, lowerBound, upperBound, hiddenColumns);

can replace method body with

this(null, lowerBound, upperBound, hiddenColumns);
See reply to comment: "They are not the same - BoundedStartRegionIterator uses visible indices and BoundedHiddenColumnsIterator uses absolute." Naming: could maybe be StartRegionIterator - compare...

See reply to comment: "They are not the same - BoundedStartRegionIterator uses visible indices and BoundedHiddenColumnsIterator uses absolute."

Naming: could maybe be StartRegionIterator - compare RangeIterator - with 'bounded' an attribute of the methods exposed rather than the class.

Agree entirely

Agree entirely

I changed the VisibleColsCollection constructor, though it makes it inconsistent with VisibleSeqsCollection (which actually needs AlignmentI)

I changed the VisibleColsCollection constructor, though it makes it inconsistent with VisibleSeqsCollection (which actually needs AlignmentI)

Renamed to be more generic. This code predates the HiddenColumns changes, and I've now updated it to take a an iterator instead of a list, which has simplified the code. It should be able to take a...

Renamed to be more generic. This code predates the HiddenColumns changes, and I've now updated it to take a an iterator instead of a list, which has simplified the code. It should be able to take a hidden ranges or visible ranges iterator which means it can do duty as either visible or hidden columns iterator (NB iterates over individual columns not ranges).

We are going around in circles here. I already more or less considered this - it is what I wanted to do when I was looking at RoaringBitmaps (which are a souped up Bitset implementation). It might ...

We are going around in circles here. I already more or less considered this - it is what I wanted to do when I was looking at RoaringBitmaps (which are a souped up Bitset implementation). It might be a lot easier to do now that HiddenColumns is substantially cleaned up but I think there are still issues to consider.

In terms of memory: Current Java implementation of Bitset uses relatively large amounts of memory with only a few entries, if those entries happen to be at the high end (see e.g. http://java-performance.info/bit-sets/ ) because it has to allocate an array to hold all the unset values up to the highest set entry. This seems to me to be quite a likely use case in Jalview. This is especially a consideration if we are going to plan to use multiple alignment-wide Bitsets (for hidden columns, column selections, gaps, ... more?)

In terms of performance: I didn't test Bitset, but RoaringBitmaps, however they are meant to be more performant so it would be surprising if Bitset were much better. Performance was largely worse or similar to the pre-existing implementation. One of the things which made the performance worse was poor performance of the cardinality calculation (relative to the implementation then, of calculating the full sum over the list of ranges) and inversion operations - both of which, unfortunately, are valuable for simplification of the code.

I do agree with you about re-use for ColumnSelection and/or gaps, probably with another iteration over HiddenColumns to get it into suitable shape. The re-use options for HiddenColumns have only really become apparent/possible with the recent clean up, or I would have treated this as a redesign rather than a refactor. I think now the plan for re-use ought to be done as a proper design task, considering the implications and possibilities, with documented decisions and discussed before implementation, rather than a post-hoc design change via code review.

sorry, hit mark as needs resolution by accident

sorry, hit mark as needs resolution by accident

Absolute column 14 is left of hidden range [15,20], so should return 15 as it does.

Absolute column 14 is left of hidden range [15,20], so should return 15 as it does.

ok

ok

done

done

done

done

Agree about not modifying the passed in BitSet parameter and will try to fix. But the hideColumns(BitSet toHide, int start, int end) is actually different to hideColumns(BitSet) as the former also ...

Agree about not modifying the passed in BitSet parameter and will try to fix. But the hideColumns(BitSet toHide, int start, int end) is actually different to hideColumns(BitSet) as the former also clears all the columns between start and end first; the latter just merges in the new hidden columns from the BitSet. Clearly that needs to be made clear in the Javadoc so I'll fix that too.

done

done

done

done

I don't think there's much can be done here. The cursor is not defined on an empty hidden columns collection, there is nothing for it to point to, so there's always going to need to be some kind of...

I don't think there's much can be done here. The cursor is not defined on an empty hidden columns collection, there is nothing for it to point to, so there's always going to need to be some kind of check somewhere when the cursor is used, either before it's called or on the return value.

(I don't buy the iterator argument, the hasNext() call is a check if the collection is empty.)

JAL-1793 update spike build to latest incl stop and synonymous variants on peptides

  1. … 28 more files in changeset.
Superseded by other changes

Superseded by other changes

Looks like this is no longer needed with numColumns now being maintained so have removed.

Looks like this is no longer needed with numColumns now being maintained so have removed.

Think that's sorted now.

Think that's sorted now.

yes I already tried variations as you've suggested - checkstyle complains (justifiably) about a variety of complexity measures depending on what I choose. I'm particularly keen not to spread out us...

yes I already tried variations as you've suggested - checkstyle complains (justifiably) about a variety of complexity measures depending on what I choose. I'm particularly keen not to spread out uses of useVisible as I think it makes the code harder to understand. I'll probably go with some submethods.

Will take a look at the third branch unit testing - it is being hit indirectly by the HiddenColumns tests already

numColumns updating done.

numColumns updating done.