[SAGA-RG] SAGA attributes as Python dictionaries?

[Ceriel Jacobs]

Andre Merzky wrote:
> Quoting [Ceriel Jacobs] (Nov 04 2009):
>> Andre Merzky wrote:
>>> Quoting [Mathijs den Burger] (Nov 04 2009):
>>>> Hi all,
>> ...
>>>> 2. Removing SAGA attributes is permanent (AFAIK when talking to Ceriel),
>>>> while a deleted dict item can be re-added latter
>>> Not sure I undestand.  the following should be valid:
>>>
>>>  if ( ! advert.attribute_exists ("foo") )
>>>  {
>>>    advert.set_attribute ("foo", "bar");
>>>  }
>>>
>>>  assert (advert.attribute_exists ("foo"), true);
>>>
>>>  advert.remove_attribute ("foo");
>>>
>>>  assert (advert.attribute_exists ("foo"), false);
>>>
>>>  advert.set_attribute ("foo", "bar");
>>>
>>>  assert (advert.attribute_exists ("foo"), true);
>> Actually, I have an issue with this: I don't think this is valid.
>>
>> Here is why, and it all depends on what the Saga specs mean
>> by "exists".
>> The notes at set_attribute say, a.o.: "the attribute is created,
>> if it does not exist" and also "only some SAGA objects allow to
>> create new attributes - others allow only access to predefined
>> attributes.  If a non-existing attribute is queried on such
>> objects, a 'DoesNotExist' exception is raised". I take this to mean
>> that ALL attributes mentioned in the SAGA specs DO exist, even
>> if they don't have a value.
>> Now, remove_attribute has as PostCond: "the attribute is not available 
>> anymore."
>> I assume this means: "the attribute does not exist anymore", what else would
>> it mean?
> 
> Absolutely correct so far, IMHO!
> 
>> So, attribute_exists() returns false on a removed attribute, but then
>> a call to set_attribute() gives a "DoesNotExist".
> 
> Here however you have two cases, as you actually quoted earlier:
> 
>> "only some SAGA objects allow to create new attributes - others
>> allow only access to predefined attributes.  If a non-existing
>> attribute is queried on such objects, a 'DoesNotExist' exception
>> is raised".
 >
> FWIW, we call the objects which allow to create new attributes
> 'extensible'.

My point was, after remove_attribute, the attribute does not exist
anymore, so, unless the object is 'extensible', the 'non-existing
attribute' clause applies and a 'DoesNotExist' should be thrown.

> So, it is 
> 
>   void saga::object::set_attribute (key, val) throws
>   {
>     switch ( extensible_ )
>     {
>       case true:
>         // always set
>         attribs_[key] = val;
> 
>       case false:
>         // object is not extensible: only set attribute if it is one
>         // of the allowed attributes
>         if ( allowed_[key].count != 0 )
>         {
>           // is a predefined attrib, can set
>           attribs_[key] = val
>         }
>         else
>         {
>           // not extensible, not predefined: error
>           throw saga::excdeption::does_not_exist ("object is not extensible, "
>                                                   "and " + key + " is not allowed");
>         }
>       }
>     }
> 
> Does that make sense to you?

It makes sense, but this is not what the specs currently say, IMHO.

>> From Mathijs, I understand that the C++ implementation has a different
>> take on what "exists" means. I may even like that interpretation better,
>> but I don't think it is supported by the current Saga specs.
> 
> I am obviously very biased, working with C++ mainly.  But at the
> moment I don't see a clash with the spec.
> 
> Earlier however I discussed with Mathijs, and agree that the spec is
> unclear about which attributes are actually optional and which are
> mandatory.  For example, 'Executable' on the job description is
> mandatory, all others on jd are optional.  It is not extensible.
> Thus, IMHO, the following should hold:
> 
>   saga::job::description jd;
> 
>   // only lists one attrib: Executable
>   assert (js.list_attributes ().count (), 1); 
>   // note: c++ is broken here as it shows an empty list - it only
>   // complains when using the jd.  I am fine with that one, too, but
>   // the spec says differently, IMHO.

This assert is wrong, I think. Various other attributes "exist", but don't have
a value. Would that be a reason for list_attributes to not list them?
The specs say:
Outputs:  keys:                 existing attribute keys

So, again, what does "exist" mean? I think that in a job_description,
the attributes "Executable", "Arguments", and all other attributes
mentioned in the specification of the job_description class "exist".
Otherwise, set_attribute could never set them without throwing
DoesNotExist.

>   // can set allowed attribs
>   js.set_attribute ("PWD", "/tmp");
>   assert (js.list_attributes ().count (), 2); 
> 
>   // empty string is valid value
>   js.set_attribute ("PWD", "");
>   assert (js.list_attributes ().count (), 2); 
> 
>   // removal removes key and val
>   js.remove_attribute ("PWD", "/tmp");
>   assert (js.list_attributes ().count (), 1); 

OK, it should certainly be one less than before.
But now, the "PWD" attribute does no longer "exist"
so another set_attribute("PWD", "...") throws
DoesNotExist, according to the specs.

>   js.set_attribute ("foo", "bar");  // throws, not allowed
> 
> 
> Basically, I think attribs should behave like a possibly
> pre-populated, a key-restricted hash table.

I agree! But the Saga specs currently say differently, I think.

> So, lets see if that makes sense to you, and others, and if that
> actually matches the spec :-)

I don't think it matches the specs :-(

Ceriel