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'. 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?

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. // 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); js.set_attribute ("foo", "bar"); // throws, not allowed Basically, I think attribs should behave like a possibly pre-populated, a key-restricted hash table. So, lets see if that makes sense to you, and others, and if that actually matches the spec :-) Thanks, Andre.

Cheers, Ceriel

-- Nothing is ever easy.

In fact it is simpler than that - as all the checking for extensible attributes and dealing with default values is done in the java or C++ so all the __setitem__ etc functions have to do is to call down to the underlying layer. Steve 2009/11/4 Steve Fisher <dr.s.m.fisher@gmail.com>:

Just a note to remind people that the dictionary syntax in python is just a convenience - you need to implement __setitem__, __getitem__, __delitem__ etc. Our __setitem__   can reject requests to set fields whoch don't already exist for those cases where the set of attributes is not extensible. It just needs a private method called by the implementation to set the attributes with any defalt values and indicate whether or not it is extensible. I think it is best to try it and see if there are any practical problems. Also see: http://docs.python.org/library/userdict.html#module-UserDict which may be useful.

Steve

Quoting [Manuel Franceschini] (Nov 04 2009):

On Wed, Nov 4, 2009 at 6:34 PM, Steve Fisher <dr.s.m.fisher@gmail.com> wrote:

...

Just a note to remind people that the dictionary syntax in python is just a convenience - you need to implement __setitem__, __getitem__, __delitem__ etc. Our __setitem__   can reject requests to set fields whoch don't already exist for those cases where the set of attributes is not extensible. It just needs a private method called by the implementation to set the attributes with any defalt values and indicate whether or not it is extensible. I think it is best to try it and see if there are any practical problems. Also see: http://docs.python.org/library/userdict.html#module-UserDict which may be useful.

It is certainly doable from the technical point of view. My question is what would be the advantages of having a dict-like interface? Would that be in this form then:

If I rember the discussion from Banff correctly, the proposal was more like jobDescription.attributes["Executable"] = "/bin/hostname" to decouple the attributes from the object properties. The dictionary seems to offer a reasonable interface for atributes, so why not use it? not sure if one can then argue why not to use jobDescription.attributes.Executable = "/bin/hostname" ? One question about both approaches though, and to also about both approaches you list below: how would async ops be realized? Best, Andre

1) jobDescription["Executable"] = "/bin/hostname")

Where 'jobDescription' is an object. Why not having just class attributes (as Mathijs showed in the first e-mail), as in:

2) jobDescription.Executable = "/bin/hostname"

IMHO, one of the most important Pythonic properties is having exactly _one_ way of achieving a task. In other words, having two different ways (even if the added way is slightly simpler) is not what I would want. Therefore, if we decide to expose one of the above ways to the end user, it should be the _only_ one, while jobDescription.set_attribute(...) should not be part of the API for the end user then.

My 2 cents.

Cheers,

-- Nothing is ever easy.

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