KarrLab / obj_tables

Tools for creating and reusing high-quality spreadsheets
https://objtables.org
MIT License
8 stars 2 forks source link

Implement context to allow object construction from strings #20

Open artgoldberg opened 6 years ago

artgoldberg commented 6 years ago

See example problem Andrew encountered and I debugged.

jonrkarr commented 6 years ago

Happy to help fix this, but I don't understand the issue. Is this a obj_model issue? This sounds an issue that can arise in downstream projects that use obj_model (e.g. wc_lang) due to a portion of the data model which isn't fully normalized (e.g. modifiers are redundantly represented in the expression and modifiers attribute). How do you want obj_model to better handle this?

On Mon, Apr 23, 2018 at 10:37 AM artgoldberg notifications@github.com wrote:

See example problem Andrew encountered and I debugged.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KarrLab/obj_model/issues/20, or mute the thread https://github.com/notifications/unsubscribe-auth/ACt2KdQRIFPecLIz6x0Gn2BSIW2Qzt89ks5trec5gaJpZM4TgBVI .

artgoldberg commented 6 years ago

Yeah, I didn't take the time to fully describe this yesterday.

The problem is that much of the initialization of some Models, at least as used in wc_lang, occurs in deserialize(), and programmatic described models, as in examples from Andrew and Ashwin don't use deserialize. E.g., see this commit to Andrew's model: https://github.com/KarrLab/h1_hesc/commit/16306c566bf340de34783d690e22b622fcc2f709#diff-a114e960f1ff4d57d99c5d511bbf74a1 This definition of a RateLawEquation fails equation=RateLawEquation(expression='CycE_type[cytosol]') because RateLawEquation.deserialize() is needed to initialize the equation modifiers. I hacked a fix, but it's not a long-term solution because it's complex because it requires that the the modeler track and provide species & compartment context for RateLawEquation.deserialize().

Fundamentally, the issue is that initialization of a Model should be distinguished from deserializing it. Initialization often needs some of the encompassing root Model, and should be able to accept a string or a parsed form for the Model. When the Model description comes from a cell in a file it's provided as a string. When it comes from a program it may be partially or completely parsed.

Programmatic descriptions should recognize the special encompassing root Model (as does PySB). A whole-cell model could be described in the context of the root Model. Instantiating individual Models can use the context to completely initialize the Model.

jonrkarr commented 6 years ago

This seems to be a misunderstanding of how to instantiate objects that are not fully normalized. Deserialization is not required to instantiate such objects. The only thing that is needed are references to the other objects. However, the user is required to update the overlapping parts of the non-normalized schema, which is annoying but not hard.

As you point out, automating the synchronization of the non-normalizing schema requires the concept of a context (e.g. the root Model or other encompassing object) which we don't currently have. This could be added. In this case, instantiation from string representations would only be possible when the user does the instantiation within a context. In turn, this would require users to use contexts as PySB requires. Otherwise, in general, errors would have to be thrown when the objects reference in the string cannot be resolved.

Currently, we have an object registry. However, this isn't quite what is needed to implement contexts. The registry could be used, but, in general, this would limit us to only 1 instantiated instance of Model at a time. Overall, that doesn't seem like a step forward, so using the registry doesn't seem like a good implementation strategy.

On Tue, Apr 24, 2018 at 10:43 AM artgoldberg notifications@github.com wrote:

Yeah, I didn't take the time to fully describe this yesterday.

The problem is that much of the initialization of some Models, at least as used in wc_lang, occurs in deserialize(), and programmatic described models, as in examples from Andrew and Ashwin don't use deserialize. E.g., see this commit to Andrew's model: KarrLab/h1_hesc@16306c5

diff-a114e960f1ff4d57d99c5d511bbf74a1

https://github.com/KarrLab/h1_hesc/commit/16306c566bf340de34783d690e22b622fcc2f709#diff-a114e960f1ff4d57d99c5d511bbf74a1 This definition of a RateLawEquation fails equation=RateLawEquation(expression='CycE_type[cytosol]') because RateLawEquation.deserialize() is needed to initialize the equation modifiers. I hacked a fix, but it's not a long-term solution because it's complex because it requires that the the modeler track and provide species & compartment context for RateLawEquation.deserialize().

Fundamentally, the issue is that initialization of a Model should be distinguished from deserializing it. Initialization often needs some of the encompassing root Model, and should be able to accept a string or a parsed form for the Model. When the Model description comes from a cell in a file it's provided as a string. When it comes from a program it may be partially or completely parsed.

Programmatic descriptions should recognize the special encompassing root Model (as does PySB). A whole-cell model could be described in the context of the root Model. Instantiating individual Models can use the context to completely initialize the Model.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/KarrLab/obj_model/issues/20#issuecomment-383958595, or mute the thread https://github.com/notifications/unsubscribe-auth/ACt2KbsiaUyC-4H0OeWMbry0Lot-WMiOks5trzn1gaJpZM4TgBVI .

jonrkarr commented 6 years ago

support filtering with pandas or similar style to pandas