-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/source description split #45
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code looks good as always, but the way the models are divided is not quite what I had in mind. I've added my thoughts here, but if you prefer, we can also discuss in person what we want to do.
|
||
|
||
class Historical(models.Model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine that this base class may come to model to things that are not strictly "historical", e.g. "heaven" as a location.
Perhaps Referent
or Entity
?
An abstract model that represents a historical entity. This may be based on one or multiple SourceDescriptions. | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had mentioned that this model provides some structure even if it doesn't have fields, but perhaps name
and description
could be added as "fundamental" fields? Just for the sake of presenting things in an interface, I think we'll need those anyway.
class LetterAction(Historical, LetterActionBase, models.Model): | ||
""" | ||
An aggregate model that represents a letter action in history. This model is based on one or multiple SourceDescriptions and other sources that are not part of this database. | ||
""" | ||
|
||
pass | ||
|
||
|
||
class LetterActionDescription(SourceDescription, LetterActionBase, models.Model): | ||
""" | ||
A description of a letter action in a source. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if there is any need for the "historical" letter action. In the case of agents, it makes sense that a description of an individual in a source will often correspond to a known historical figure, and that these historical figures are a useful "entry point" into the database. Cross-referencing individual actions is much harder, if ever possible, and less useful for making queries.
I would argue that while a SourceDescription
is often referring to something "real" (e.g. a person, an action, a place), it's not always useful to represent both layers in the database, if they don't serve the analysis. (In the case of actions, it is also tricky that actions in texts do not necessarily represent "real" events but may serve a literary purpose.)
class Agent(Historical, AgentBase): | ||
""" | ||
An aggregate model that represents an agent (person or group) in history. This model is based on one or multiple SourceDescriptions and other sources that are not part of this database. | ||
""" | ||
|
||
pass | ||
|
||
|
||
class AgentDescription(SourceDescription, AgentBase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also applies to other models. In most cases, information that is represented in the SourceDescription
does not need to be duplicated in the corresponding Historical
model. It's more precise to display or query the agent through its descriptions.
I would say that an Agent
may also include information of its own, but mainly things that are not really connected to sources, and more about representing the agent in the interface.
For example, the Agent
Radegund could include a name, longer description, and an image that can be used in the interface. Then an AgentDescription
of Radegund describes the language used in source texts to describe her gender, social status, names used in the text, etc.
return f"{super().__str__()} (described in {self.source})" | ||
|
||
|
||
class Gift(Historical, GiftBase, models.Model): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing as for actions: I think we can start out without a Historical
model for gifts and letters, though we can add that layer of analysis when needed.
class SpaceDescription(Historical, SpaceDescriptionBase, models.Model): | ||
""" | ||
An aggregate model that represents a space in history. This model is based on one or multiple SourceDescriptions and other sources that are not part of this database. | ||
""" | ||
|
||
pass | ||
|
||
|
||
class SpaceDescriptionDescription( | ||
SourceDescription, SpaceDescriptionBase, models.Model | ||
): | ||
""" | ||
A description of a space description in a source. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"A description of a space description"?
This distinction is not really appropriate here; as this awkward docstring highlights, SpaceDescription
was already a type of description.
In the case of spaces, as I understand it, source texts are so different in what they describe, and so vague, that it is a futile exercise to try to link specific places between sources. This is why we ended up with a model where a single SpaceDescription
contains several aspects that may have been described, each of which may link to a historical reference point, such as a kingdom or diocese.
So to map the space
app to the Historical
/Description
division... When I wrote this originally, I imagined we would divide it like this:
PoliticalRegion
,EcclesiasticalRegion
, etc., are allHistorical
modelsSpaceDescription
is aSourceDescription
Alright, it does seem I have misunderstood a couple of things; let's discuss the necessary refactors soon! |
This PR introduces a split between actual, 'historical' entities and source descriptions (closes #32 ). The implementation is as follows.
Feel free to take over this PR and push commits on it while I am away, if it blocks your progress.
There are two abstract models, as per your suggestion:
Historical
is an empty abstract class. It marks historical entities and we can use it to later implement fields that are common to all of them, if needed.SourceDescription
contains fields that used to be in theReference
model and is inherited by source description models. As far as I can see, this class now renders theReference
model obsolete, so I removed it.The idea is that each entity is represented by three model objects (taking
Agent
as an example).AgentBase
is the base model that contains all relevant content fields. Also all ForeignKeys (for instance onAgentName
) should refer to this model. The end user should not see this, and it has not been included in the Django admin interface.Agent
inherits fromHistorical
and represents the actual 'aggregated' historical entity in our knowledge base. (We need a better term for this 😅).AgentDescription
inherits fromAgentBase
andSourceDescription
and includes atarget
field that links it to theAgent
. This replaces theGenericForeignKey
that existed on theReference
model.This split has been implemented for the
Agent
,Letter
,Gift
,SpaceDescription
andLetterAction
models.Admin interface
In the
![image](https://private-user-images.githubusercontent.com/73882506/316308802-c145b443-2861-4fca-bfe5-c58342cedadc.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1OTM5MTQsIm5iZiI6MTczOTU5MzYxNCwicGF0aCI6Ii83Mzg4MjUwNi8zMTYzMDg4MDItYzE0NWI0NDMtMjg2MS00ZmNhLWJmZTUtYzU4MzQyY2VkYWRjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDA0MjY1NFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk3NjNlYzU3YzMzNDdmMWE3MTlmNGVjNjVkYjUxNDJlYmRkNjdhZDZiZWY4NmFjODk1NTFiNmFlNDBlZmUxZjUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.pcBFlCq2g28AgtHUO6bKr7Xqs2ccBBpXZjA_lrGEmvE)
AgentDescription
admin interface, we can easily select a target for a description:In the
![image](https://private-user-images.githubusercontent.com/73882506/316308835-f95b9971-75e3-4b6f-b988-4daa14774906.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1OTM5MTQsIm5iZiI6MTczOTU5MzYxNCwicGF0aCI6Ii83Mzg4MjUwNi8zMTYzMDg4MzUtZjk1Yjk5NzEtNzVlMy00YjZmLWI5ODgtNGRhYTE0Nzc0OTA2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDA0MjY1NFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWU5ZmZiYmQ4NjA5Mzc5N2I1OGVkMmU5NjdkYTM0YzkwMWViZDMwNzIxNGIzZWJjMGI4OTFmZjE2OWMzZDkzYjgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.3h98ZF4cOkjjYThBZ6ynNzCkn42IlaZRJXqEq_DYfco)
Agent
admin interface, connected descriptions are shown in a semi-custom tabular inline.Other issues
This PR also includes a migration that is currently missing in develop.