Drasil: Data definition data structure?

[niazim3]

Continuing the conversation from b032040, a proper data-structure for data definitions are needed. @JacquesCarette How about something similar to InstanceModel and TheoreticalModel
https://github.com/JacquesCarette/Drasil/blob/b3938fb5e7013e5165a0be8c1d0641bf08a7f10f/code/Language/Drasil/Chunk/InstanceModel.hs#L32-L44
? So:

-- | A data definition is a QDefinition that may have additional notes. 
-- It also has attributes like derivation, source, etc.
data DataDefinition = DD { _qd :: QDefinition
                         , _notes :: Maybe [Sentence]
                         }
makeLenses ''InstanceModel

(where QDefinition is https://github.com/JacquesCarette/Drasil/blob/b3938fb5e7013e5165a0be8c1d0641bf08a7f10f/code/Language/Drasil/Chunk/Eq.hs#L24-L33 with the _notes field removed)

[JacquesCarette]

Yes, let's start with that. And, in fact, I think I would then like to move everything else to that. QDefinition should really just be a Quantity with a defining equation. The only thing I think we could leave (upgrade?) is to go from ShortName to having a Label. Because sometimes an anonymous definition that can be referred to is still useful.

[niazim3]

To clarify, you want QDefinition's References and Derivation to be moved into the newly created DataDefinition type, correct? @JacquesCarette

[JacquesCarette]

Yes.

[niazim3]

@JacquesCarette I was planning on removing QDefinition's following fields next

            _ref :: References
           , _deri :: Derivation 
           , _refName :: ShortName  --(Label in the addLabel branch)
           , _notes :: Maybe [Sentence] --(from Master, can easily be removed)

and updating all the Examples to use DataDefinitions accordingly, however, it seems like a really big change, especially considering that #695 has already updated the QDefinition data type to have a Label.

Should this issue be worked on after the referencing issues have been closed with the complete implentation of LabelledContent and hasLabel, hasShortName, and Referable class implementation for chunks?

If this can be worked on parallel to those branches/issues, would it be possible to suggest a workflow please?

[szymczdm]

If you start by creating the new datatype, then slowly swapping things over, and then remove the unnecessary QDefinition fields it will likely be the easiest way to do things.

[szymczdm]

Oops, didn't see the last bit of questions, I'd say this can be done in parallel, but you'll need to stay on top of merging in master to keep things clean.

[niazim3]

Just going to note here that DataDefinition only temporarily expects a ShortName instead of a Label (as of commit 825b46f0fd4ad4b). This was done so changes can be merged into master quicker (in case work on referencing takes longer to implement). If that doesn't turn out to be the case, this data type can be updated following the examples of data type updates tracked in #695.

[niazim3]

I'm currently stuck on an error (please see e858149 for error message and problems).
If it was incorrect to attempt to replace calls to [QDefinition] for GlassBR (because it led to a lot of code duplicaition in an attempt to get an output), the commit mentioned above can be deleted to just keep the changes until 2e09003.

[JacquesCarette]

The 'correct' thing to do is to replace QDefinition with DataDefinition one-by-one, focusing on the ones that don't break anything. Then file issues for the cases that break.

[niazim3]

Update:

As of commit 108749181, GlassBR's Data Definitions are displayed using the DataDefinition type. Something that had to be done however, to avoid a conversion error mentioned in commit e7616a8e1, was pass the SystemInformation a list of the QDefinitions that the DataDefinitions are built off of.

1. This may be alright since the tmodels and imodels that are passed into the SystemInformation are also the "building blocks" of TheoryChunks and InstanceModels; that is RelationConcept. Since the building blocks are passed into the SystemInformation for GlassBR's theoretical and instance models, is it alright for GlassBR's data definitions' "building blocks" of QDefinition to also be passed in instead of attempting to convert the DataDefinitions? @JacquesCarette @szymczdm

2. If the above is going on the right track, do you prefer having a list of QDefinitions for each of the Examples (like commit 108749181 does) or using some getter that'll extract the QDefinition from each DataDefinition so there's only 1 list of DataDefinitions instead of a list of QDefinitions and a list of DataDefinitions for each example? @JacquesCarette @szymczdm

---

To do:

It's mentioned in the comments of commit 2071585 that we need to

• [x] add some kind of 'Scope' field to (at least) DataDefinition

[JacquesCarette]

1. It would be better to have DataDefinitions in SystemInformation than QDefinition. However, to do this incrementally, it is probably best to add a new field and deprecate the old one when all the examples are converted. I guess the code should look at the field containing the DataDefinition first, it is it not [], use it (and only it), otherwise use the old field (whether it is [] or not).
2. A getter and only 1 list.

[niazim3]

Should any other feature/updates be made to the DataDefinition data type before the rest of the examples migrate from using QDefinitions to DataDefinitions?

[JacquesCarette]

I am not seeing anything that needs to be added at the moment. We might find out during the migration!

[niazim3]

This issue can be closed as the examples have migrated to use DataDefinition. All that's left related to this issue's contents is removing the now no longer used fields for QDefinition, which will be an issue tracked in #973.