Re: [agtk-devel] AGAPI 2.0b1: GetAnchorSet returns unordered set

[Matthias T. <th...@ei...>]

Hello Haejoong,

I am a little confused :)

Haejoong Lee wrote:
> So you are looking for a function which returns a time-ordered anchor 
> list?  Then, you can use GetAnchorSetByOffset.

Well, but only if I set the epsilon parameter to infinity? ;)

set<AnchorId> GetAnchorSetByOffset(AGId agId,Offset offset, double 
epsilon=0.0)

I thought this function was to get the anchors at a certain offset and 
with epsilon distance around this offset, but not all anchors?



However, in the implementation of GetAnchorSet in AGAPI.cc:

set<AnchorId>
GetAnchorSet(AGId agId)
{
   try {
     AnchorSeq* aidset = Identifiers::getAGRef(agId)->getAnchorSet();
     set<AnchorId> aids;

     for (AnchorSeq::iterator pos = aidset->begin(); pos != 
aidset->end(); ++pos)
       aids.insert((*pos)->getId());
     return aids;

   } catch (AGException e) {
     logError("GetAnchorSet",e);
   }
}

the anchors are collected from getAnchorSet() in AG.h:

   /// Get the set of anchors, sorted by offsets.
   AnchorSeq* getAnchorSet() { return &I.anchorSet; }

So this anchor *sequence* is already sorted by offsets and by ids iff 
offsets are the same, as seen in Anchor.h

/**
  * Anchor compare function.
  * A function to compare anchors by first comparing their offsets,
  * then their ids if the offsets are the same.
  **/
class AnchorCompFunc {
  public:
   bool operator() (const Anchor* a1, const Anchor* a2) const {
     return *a1 < *a2;
   }
};

/**
  * Anchor Seqquence.
  * a set of anchors sorted by AnchorCompFunc, i.e. offsets
  **/
typedef set<Anchor*,AnchorCompFunc> AnchorSeq;


So everything is already there, only the API function in AGAPI.cc should 
return list<AnchorId> instead of set<AnchorId>


> By the way, I just found two problems with ...SetByOffset functions.  
> (There are two ...SetByOffset functions: GetAnchorSetByOffset and 
> GetAnnotationSetByOffset.) It would have been better if the name was 
> ...ListByOffset.  The real problem is that in C++ implementation they 

For GetAnnotationSet this is already there:

set<AnnotationId> GetAnnotationSetByOffset(AGId agId,Offset offset, 
const AnnotationType& type="");
list<AnnotationId> GetAnnotationSeqByOffset(AGId agId, Offset begin=0.0, 
Offset end=0.0, const AnnotationType& type="");

So if you introduce a complementary ..Seq.. function for Anchors, I 
would also be happy. Still I think that GetAnchorSet should return a 
list (and maybe be renamed to GetAnchorSeq).

> return set<...> instead of list<...>.  This means what you get from 
> these functions is actually a set of ids ordered by ids, not offsets!

Yes, that's exactly my problem ;)

> I've updated the CVS repository with the fixes; they return lists now.

I just checked out the Toward-2_0 branch, and cannot find the changes. 
In fact, I copied the above code from the sources just checked out.

> This change doesn't affects Python and Tcl interfaces.  Thanks for 
> making me realize those problems.

You're welcome :)

> Finally, the SWIG interface files are available from the CVS:
> 
>    $ cvs -d:pserver:ano...@cv...:/cvsroot/agtk login
>    [return at the login prompt]
>    $ cvs  -d:pserver:ano...@cv...:/cvsroot/agtk co \
>          -r Toward-2_0 AGLIB/src/ag_wrapper

Thank you, I didn't realize that you made a branch and was already 
wondering if you stopped updating the CVS ;)

Regards.
Matthias


P.S. By the way, I have submitted two "Bug reports".