ModelSEED / ModelSEED

Other
3 stars 2 forks source link

Name of biochemistry object not being updated? #156

Open samseaver opened 11 years ago

samseaver commented 11 years ago

This threw me for a while. Since part of what I'm doing involves consolidating a biochemistry and saving it as a new object, for instance, I consolidate KEGG into a new biochemistry called "Cns_KEGG", I'm using this code in my commands:

if (defined($opts->{saveas})) {
        my $new_ref = $helper->process_ref_string($opts->{saveas}, "biochemistry", $auth->username);
        verbose("Saving biochemistry with merged compounds as ".$new_ref."...\n");
        $store->save_object($new_ref,$new_biochemistry);
}

The problem is, though you save as a new biochemistry object, it retains its old name. So calling $biochem->name() for the new 'Cns_KEGG' object returns 'KEGG'.

I understand that Store::save_object() acts on any object generically, but for the instance of biochemistry objects, I need to call $biochem->name('Cns_KEGG') before saving it. I wanted to make the point that this may lead to confusion in the future.

samseaver commented 11 years ago

I can confirm that this code:

if (defined($opts->{saveas})) {
        my $new_ref = $helper->process_ref_string($opts->{saveas}, "biochemistry", $auth->username);
        verbose("Saving biochemistry with merged compounds as ".$new_ref."...\n");
        $new_biochemistry->name("New_Name");
        $store->save_object($new_ref,$new_biochemistry);
}

only works within the lifetime of the script. In other words, it'll print out the correct name before or after saving it, but once the script has exited, and I run another one, the new name is lost. So somehow, if I've got this right, using $store->save_object() doesn't preserve the updated name for future use. Am I missing something?

cshenry commented 11 years ago

The name must be reserved with the save. Are you sure of this behavior? Frankly, name probably shouldn't be in the attributes of the object itself, but rather, it should be set from the alias on load.

Sent from my iPad

On Nov 15, 2012, at 4:53 PM, samseaver notifications@github.com wrote:

I can confirm that this code:

if (defined($opts->{saveas})) { my $new_ref = $helper->process_ref_string($opts->{saveas}, "biochemistry", $auth->username); verbose("Saving biochemistry with merged compounds as ".$new_ref."...\n"); $new_biochemistry->name("New_Name"); $store->save_object($new_ref,$new_biochemistry); } only works within the lifetime of the script. In other words, it'll print out the correct name before or after saving it, but once the script has exited, and I run another one, the new name is lost. So somehow, if I've got this right, using $store->save_object() doesn't preserve the updated name for future use. Am I missing something?

— Reply to this email directly or view it on GitHub.

cshenry commented 11 years ago

But I would not be setting name where you are setting it... That's creating inconsistent behavior. Why are you so worried about the name attribute anyhow. What's the use case here?

Sent from my iPad

On Nov 15, 2012, at 4:53 PM, samseaver notifications@github.com wrote:

I can confirm that this code:

if (defined($opts->{saveas})) { my $new_ref = $helper->process_ref_string($opts->{saveas}, "biochemistry", $auth->username); verbose("Saving biochemistry with merged compounds as ".$new_ref."...\n"); $new_biochemistry->name("New_Name"); $store->save_object($new_ref,$new_biochemistry); } only works within the lifetime of the script. In other words, it'll print out the correct name before or after saving it, but once the script has exited, and I run another one, the new name is lost. So somehow, if I've got this right, using $store->save_object() doesn't preserve the updated name for future use. Am I missing something?

— Reply to this email directly or view it on GitHub.

samseaver commented 11 years ago

Again, its about the messages a command prints, in MS::Biochemistry::mergeBiochemistry, I was having it print out a verbose message like this:

Merging n compounds from KEGG with n from Cns_KEGG

Since I'm "chaining" merges, and saving to a new biochemistry object, the name remains 'KEGG', and never gets updated for the new objects. This had me confused for a while. It's not necessarily important, but if users were to use mergebio several times in succession in building their final biochemistry object, then the output biochemistry name would make them think they're using the wrong object.

cshenry commented 11 years ago

Well, I think you have grasped onto a really important point here. Ultimately, I'm inclined to strip the "name" attribute out of every provenance object as stored in the DB, but have the database add these attributes when you load and delete these attributes when you save. What we want to guarantee is that the name is completely in sync with the database alias under which the object is saved. If we do this, I would make name a read only attribute, because the developer should never be manually changing the name.

On Nov 16, 2012, at 9:00 AM, samseaver notifications@github.com wrote:

Again, its about the messages a command prints, in MS::Biochemistry::mergeBiochemistry, I was having it print out a verbose message like this:

Merging n compounds from KEGG with n from Cns_KEGG

Since I'm "chaining" merges, and saving to a new biochemistry object, the name remains 'KEGG', and never gets updated for the new objects. This had me confused for a while. It's not necessarily important, but if users were to use mergebio several times in succession in building their final biochemistry object, then the output biochemistry name would make them think they're using the wrong object.

— Reply to this email directly or view it on GitHub.

devoid commented 11 years ago

+1 on this. We can't "guarantee is that the name is completely in sync with the database alias under which the object is saved." However, we can guarantee that the name corresponds with what the database originally gave us.

Making it read-only is also a good idea. Since the database ought to ignore it when saving.