Review and discuss the first refactorings of models.py

[bhaugen]

Looking at https://github.com/django-rea/nrp/compare/models_refactor

Looks like it breaks down somewhat into layers:

• core: looks like the "Reality" or "What has occurred" layer of REA. See layer diagrams below.
• types: the "Recipe" or "What could be" layer.
• schedule: part of the "Plan" or "What is scheduled" layer.
• processes: overlap between "Plan"/"What is scheduled" and "Reality"/"What has occurred"
• trade: separating out Exchanges
• behavior: value equations and distributions plus facets and patterns
• misc: what it is

NRP layers:

Bill McCarthy's REA layers:

@escanda great work! Let me know if I summarized correctly. Will start reviewing and discussing in comments.

[bhaugen]

I'll start with some questions and analysis.

@escanda what were your goals for this refactoring? What did you want to accomplish? And what did you not want to accomplish?

I can think of several goals for refactoring models:

• smaller and less intimidating models files.
• separating into units of work: that is, groups of model classes that somebody might work on together, that might be logically connected in features.
• separating into modules that one or other group might want to use, or not use (which might be done better by separating into apps than just models files).
• separating into components which somebody might want to override with their version of the same component.
• other goals? I am sure I have not thought of everything.

When @fosterlynn and I discussed refactoring in a long car ride, I thought of a table, like this, with rows for the layers and the columns for the R, E, and A, and adding Process and Exchange:

Layer	Resource	Event	Agent	Process	Exchange
Recipe/Type	ResourceType	EventType, CommitmentType	AgentType	ProcessType	ExchangeType
Plan		Commitment		Process	Exchange
Reality	Resource	EconomicEvent	Agent	Process	Exchange

That's a simplified table and omits a lot of the "supporting" classes that would need to go with the majors. And note, Process and Exchange appear on two layers.

But it would be possible to refactor by layer (which is pretty much what @escanda did, if I understood correctly), or by column, or by both (radical refactoring into possibly the smallest practical modules).

And it would be possible to refactor according to other criteria ( @fosterlynn is saying "functionality" off to the side right now...;-)

I am not advocating anything here, just thinking in public. Before advocating, I want to know more about which modules would make sense for the three groups involved in the code base now: Freedom Coop (OCP), GoPacifia (DEEP), and Matrioshka (OCP for now). That would be another reason for factoring out features that some groups don't want to use.

[fosterlynn]

And it would be possible to refactor according to other criteria ( @fosterlynn is saying "functionality" off to the side right now...;-)

... but not as a recommendation, just to say there is probably some sweet spot between data driven and functionality driven refactoring. I don't know what it is in this case. I'd like to spend some focused time on it, hoping to do that today.

Before advocating, I want to know more about which modules would make sense for the three groups involved in the code base now

👍

[fosterlynn]

OK, here is my first attempt to do a combined resource driven and function driven set of groupings on the data side. Could be very wrong, I think a lot of this will come out more clearly when we try to do the views too. But at least hopefully some food for thought.

#agent with type info, could create a separate agent app as well as support all of NRP
from django_rea.valueaccounting.models.agent import (
    EconomicAgent,
    AgentUser, #should this be here? not sure how we are separating out the account stuff
    AgentAssociation,
    AgentType,
    AgentAssociationType,
)

#could support a separate app for designs, as well as just what is needed for most of NRP - 
#but decided to put exchange here too because eventually we want to have combined 
#process and exchange recipes
from django_rea.valueaccounting.models.recipe import (
    EconomicResourceType,
    ResourceClass,
    ResourceTypeSpecialPrice,
    CommitmentType,
    EventType,
    ProcessType,
    ResourceTypeList,
    ResourceTypeListElement,
    Feature,
    Option,
    SelectedOption, #double check this is part of options
    AgentResourceType, #not sure if this appears anywhere on the UI now, and too bad to put it
                # here, because now agent becomes required - maybe we can eliminate it
    Unit,
    ExchangeType,
    TransferType,
)

#could support a resource sharing app, as well as part of the core layer of NRP
from django_rea.valueaccounting.models.resource import (
    EconomicResource,
    ResourceState,
    AgentResourceRole,
    AgentResourceType,
    AgentResourceRoleType,
)

#could combine this and process and exchange - but I think on the view/templace 
#side it is good to separate process and exchange
from django_rea.valueaccounting.models.event import (
    EconomicEvent,
    CachedEventSummary,
    EventSummary,
    AccountingReference,
)

from django_rea.valueaccounting.models.process import (
    Process,
)

from django_rea.valueaccounting.models.trade import (
    Exchange,
    Transfer,
)

from django_rea.valueaccounting.models.schedule import (
    Commitment,
    Reciprocity, #possibly not used?
    Order,
)

#only needed if you are doing contributory accounting
from django_rea.valueaccounting.models.distribution import (
    Distribution,
    DistributionValueEquation,
    ValueEquation,
    ValueEquationBucket,
    ValueEquationBucketRule,
    IncomeEventDistribution,
    Claim,
    ClaimEvent,
)

#both process and exchange facet-value config
from django_rea.valueaccounting.models.facetconfig import (
    Facet,
    FacetValue,
    ResourceTypeFacetValue,
    PatternFacetValue,
    ProcessPattern,
    PatternUseCase,
    TransferTypeFacetValue,
    UseCase, #this may need to go somewhere more general, but I think this is most if not all of its usage
    UseCaseEventType,
)

#needed in both agent and resource
from django_rea.valueaccounting.models.location import (
    Location,
)

from django_rea.valueaccounting.models.misc import (
    HomePageLayout,
    Help,
)

[bhaugen]

Reciprocity, #possibly not used?

Yeah, it's dead, let's remove it.

[ghost]

But it would be possible to refactor by layer (which is pretty much what @escanda did, if I understood correctly),

I must confess I took Pavel's book and figured out some of the naming conventions. I must say, I like a lot @fosterlynn 's approach. I am a REA noob so please bear with me :) I must dig deeper into REA modelling.

Before advocating, I want to know more about which modules would make sense for the three groups involved in the code base now: Freedom Coop (OCP), GoPacifia (DEEP), and Matrioshka (OCP for now). That would be another reason for factoring out features that some groups don't want to use.

Imho @fosterlynn initial proposal is really solid.

[bhaugen]

What would you all think of separating the models into their own app(s) and the views and templates into another app or apps? That way, each group could create their own UI, to the extent that they wanted to do so? We'd need to provide a default UI so new groups could get up and running with a complete-ish system.

The API is already its own app, but might want to be split up too, depending on how we split the models. (I think the API is dependent on the models, but not on the views.)

@fosterlynn reminds me that we might have some logic in the views that should really be in the models or utils, so we might need to do some cleanup.

[fosterlynn]

we might have some logic in the views that should really be in the models or utils, so we might need to do some cleanup.

And by "might" I mean I'm sure we do. There's even a little hidden in the forms. But I think this can all or mostly be later in the refactoring process, first would be to get it split up and running as is, and get it up to the latest Django.

[fosterlynn]

I am a REA noob so please bear with me :) I must dig deeper into REA modelling.

@escanda you are doing amazing! ❤️ Some of this is also NRP specific (beyond REA), sometimes for good reasons, sometimes for just historical / get-it-out-there / lots-of-experimenting kinds of reasons.

I am really pleased you are pushing on this refactoring, it is exciting to think of this getting a good clean up! And more exciting to think that it could be a viable open source project for multiple coordinating groups! 😄

[ghost]

If everyone is okay with @fosterlynn 's proposal, I can take on the task of accommodating models into this new structure in 4 hours approximately. I will, nonetheless, instead of following the initial radical-black-boxed refactoring approach, post updates here so you can review, and finally create a pull request for final review before merging into master.

[bhaugen]

See also #2

[fosterlynn]

If everyone is okay with @fosterlynn 's proposal, I can take on the task of accommodating models into this new structure in 4 hours approximately.

If we use ABCs, do we even want to split the models? Or would that be in the next "layer"? (I don't know the answer.)

[bhaugen]

@escanda

instead of following the initial radical-black-boxed refactoring approach,

Yeah, don't go radical (I assume you mean all the cells in that table above), or at least not yet.

[fosterlynn]

I can take on the task of accommodating models into this new structure in 4 hours approximately.

Other question, should we do some Django upgrades first? (Just asking, I am more or less following but not technical enough to have an opinion. It does seem like we need to make a few technical decisions first though?)

[ghost]

Sorry, I just came back to my computer, I am starting in a few minutes.

If we use ABCs, do we even want to split the models? Or would that be in the next "layer"? (I don't know the answer.)

We want. And furthermore, we want to follow a similar schema to the models naming convention once we start splitting things into apps.

Yeah, don't go radical (I assume you mean all the cells in that table above), or at least not yet.

I won't :)

Other question, should we do some Django upgrades first? (Just asking, I am more or less following but not technical enough to have an opinion. It does seem like we need to make a few technical decisions first though?)

Totally right :) That's why I proposed my hidden agenda openly in here. For a first iteration, that is, using your naming and file conventions, we don't need yet to upgrade Django. But in the near future, once we start splitting into apps, we do.

[numapanuma]

Hi dears! What was the reciprocity function? Wasn't it the "counter transfef in an exchange event? El feb 28, 2017 3:58 PM, "Bob Haugen" <notifications@github.com> escribió:

…

Reciprocity, #possibly not used? Yeah, it's dead, let's remove it. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB9Y83peMbQrlxeSinLaINROxkIx8Q4Fks5rhG5dgaJpZM4MOeJe> .

[numapanuma]

It looks good to me Lynn. I can easily visualize these small apps doing the job. El feb 28, 2017 3:55 PM, "Lynn Foster" <notifications@github.com> escribió:

…

OK, here is my first attempt to do a combined resource driven and function driven (but tending towards function) set of groupings on the data side. Could be very wrong, I think a lot of this will come out more clearly when we try to do the views too. But at least hopefully at least some food for thought. #agent with type info, could create a separate agent app as well as support all of NRP from django_rea.valueaccounting.models.agent import ( EconomicAgent, AgentUser, #should this be here? not sure how we are separating out the account stuff AgentAssociation, AgentType, AgentAssociationType, ) #could support a separate app for designs, as well as just what is needed for most of NRP - but decided to put exchange here too because eventually we want to have combined process and exchange recipes from django_rea.valueaccounting.models.recipe import ( EconomicResourceType, ResourceClass, ResourceTypeSpecialPrice, CommitmentType, EventType, ProcessType, ResourceTypeList, ResourceTypeListElement, Feature, Option, SelectedOption, #double check this is part of options AgentResourceType, #not sure if this appears anywhere on the UI now, and too bad to put it here, because now agent becomes required - maybe we can eliminate it Unit, ExchangeType, TransferType, ) #could support a resource sharing app, as well as part of the core layer of NRP from django_rea.valueaccounting.models.resource import ( EconomicResource, ResourceState, AgentResourceRole, AgentResourceType, AgentResourceRoleType, ) #could combine this and process and exchange - but I think on the view/templace side it is good to separate process and exchange from django_rea.valueaccounting.models.event import ( EconomicEvent, CachedEventSummary, EventSummary, AccountingReference, ) from django_rea.valueaccounting.models.process import ( Process, ) from django_rea.valueaccounting.models.trade import ( Exchange, Transfer, ) from django_rea.valueaccounting.models.schedule import ( Commitment, Reciprocity, #possibly not used? Order, ) #only needed if you are doing contributory accounting from django_rea.valueaccounting.models.distribution import ( Distribution, DistributionValueEquation, ValueEquation, ValueEquationBucket, ValueEquationBucketRule, IncomeEventDistribution, Claim, ClaimEvent, ) #both process and exchange facet-value config from django_rea.valueaccounting.models.facetconfig import ( Facet, FacetValue, ResourceTypeFacetValue, PatternFacetValue, ProcessPattern, PatternUseCase, TransferTypeFacetValue, UseCase, #this may need to go somewhere more general, but I think this is most if not all of its usage UseCaseEventType, ) #needed in both agent and resource from django_rea.valueaccounting.models.location import ( Location, ) from django_rea.valueaccounting.models.misc import ( HomePageLayout, Help, ) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB9Y8z4mhRA8J7J-bhtbhJx1lCT5MxWuks5rhG2TgaJpZM4MOeJe> .

[bhaugen]

What was the reciprocity function? Wasn't it the "counter transfef in an
exchange event?

No, it was a previous attempt to do the same thing. Never got used. The redesign of exchanges makes it totally obsolete.