Diff for "RFC_1/RFC1_Comment_API"

Describe RFC 1/RFC1 Comment API here.

IlianaAvila -9/30/05- I put in this new comment section, since I had additional comments that did not fit in any existent category.

IlianaAvila -9/30/05- I think we also need:

/**
 * Deletes the given attribute completely for all objects
 */ 
boolean deleteAttribute (String attributeName);

The implementation for this method can be more efficient than calling deleteAttribute (String id, String attributeName) for all ids. But I don't know how attributes are stored, so maybe it would be the same.

NeriusLandys -9/30/05- See CyDataDefinition.undefineAttribute(String).

IlianaAvila -9/30/05- I think the method should be in CyAttributes, and called deleteAttribute(String). The name undefineAttribute seems strange.

IlianaAvila -9/30/05- I am not sure that I like to have the additional CyData and CyDataDefinition interfaces. I like the idea of having one single interface to handle attributes for Cytoscape. What additional methods do these interfaces have? Are they really going to be used? Could we put the needed ones in CyAttributes?

NeriusLandys -9/30/05- I think Ethan's API is fairly intuitive and would definitely be a cinch to implement.

GaryBader - 9/30/05 - If we add deleteAttribute to permanently remove an attribute from Cytoscape, which I think we would, I think the deleteAttribute method that is in Ethan's current proposal should be called clearAttribute.

EthanCerami - 10/1/05 - I think Iliana's suggestion re: deleteAttribute(String attributeName) is a good one, and I think this is currently available via GraphObjAttributes. However, having one method called clearAttribute and one called deleteAttribute seems a bit confusing. I think they should both be named deleteAttribute(), e.g. deleteAttribute (String id, String attributeName) and deleteAttribute (String attributeName).

GaryBader - 10/1/05 - Good point. It seems that we use the word 'attributes' for a set of attributes in the manual, and 'attribute' for just one attribute. So deleteAttribute (String attributeName) should probably be deleteAttributes (String attributeName)

IlianaAvila -10/3/05 - I have to ask again, is everyone OK with the existence of CyData? CyDataDefiniton? I have not looked at the interfaces, but, I think that 1 class should be enough. We can talk about this on the conference call today.

RowanChristmas 10/03/05 -- CyData/CyDefenition provide a much more powerful API. The methods we are discussing are just a subset of the functionality they provide. Also I think that "getClass" should be "getType" and return one of the defined byte types.

EthanCerami - 10/4/05 - OK, I changed getClass to getType, and it now returns byte values. Revised API will be posted soon.

IlianaAvila - 10/5/05 - I have two questions about the new API.

1. In the Javadoc, it is stated that the attributes are attached to unique identifiers which can be obtained through CyNode.getIdentifier/CyEdge.getIdentifier. I am used to using the CANONICAL_NAME value instead, which I get from Cytoscape.getNodeAttributeValue(node, Semantics.CANONICAL_NAME). Are they the same thing?

2. What is the difference between CyAttributes.resetAttribute and CyAttributes.deleteAttribute? It seems to me that there is no difference. I think we should only have CyAttributes.deleteAttribute.

EthanCerami - 10/5/05 - On your two questions:

1. I am not actually sure, and don't understand the BioDataServer well enough to answer definitively. However, it looks as if when you create a new node via Cytoscape.getCyNode(), there is a call to Semantics.assignNodeAliases(), and this code seems to set the node identifier and it's canonical name to the same String value. Here is an excerpt from the code:

   1 String name = node.getIdentifier().toUpperCase();
   2 String cname = bds.getCanonicalName( species, name );
   3 // name was not canonical name
   4 if ( name != cname )
   5   node.setIdentifier( cname );
   6 Cytoscape.setNodeAttributeValue( node, CANONICAL_NAME, cname );
   7 

2. Good point, and I think we already agreed to this change earlier in the thread. Revised API will now look like this:

   1     /**
   2      * Deletes the id/attributeName pair.
   3      *
   4      * @param id            unique identifier.
   5      * @param attributeName attribute name.
   6      * @return true indicates attribute was
   7      *         successfully removed.
   8      */
   9     public boolean deleteAttribute(String id, String attributeName);
  10 
  11     /**
  12      * Deletes the specified attribute.
  13      * <P>
  14      * Calling this method deletes all id/attributeName pairs with this
  15      * attributeName, and resets the specified attribute data type to:
  16      * TYPE_UNDEFINED.  Please use with caution!
  17      *
  18      * @param attributeName attribute name.
  19      * @return true indicates attribute was successfully reset.
  20      */
  21     public boolean deleteAttribute(String attributeName);
  22