JacquesCarette / Drasil

Generate all the things (focusing on research software)
https://jacquescarette.github.io/Drasil
BSD 2-Clause "Simplified" License
142 stars 26 forks source link

Preference for Label Display #970

Closed niazim3 closed 5 years ago

niazim3 commented 6 years ago

The logs in the addLabels branch have many discrepancies like the following

6378c6378
< <a href=#Table:InDataConstraints>Table:InDataConstraints</a> and <a href=#Table:OutDataConstraints>Table:OutDataConstraints</a> show the data constraints on the input and output variables, respectively. The column for physical constraints gives the physical limitations on the range of values that can be taken by the variable. The uncertainty column provides an estimate of the confidence with which the physical quantities can be measured. This information would be part of the input if one were performing an uncertainty quantification exercise. The constraints are conservative, to give the user of the model the flexibility to experiment with unusual situations. The column of typical values is intended to provide a feel for a common scenario.
---
> <a href=#Table:InDataConstraints>InDataConstraints</a> and <a href=#Table:OutDataConstraints>OutDataConstraints</a> show the data constraints on the input and output variables, respectively. The column for physical constraints gives the physical limitations on the range of values that can be taken by the variable. The uncertainty column provides an estimate of the confidence with which the physical quantities can be measured. This information would be part of the input if one were performing an uncertainty quantification exercise. The constraints are conservative, to give the user of the model the flexibility to experiment with unusual situations. The column of typical values is intended to provide a feel for a common scenario.

Generated:

image

Stable:

image

Is there a preference for either display?

JacquesCarette commented 6 years ago

Definitely prefer Generated. The Table prefix should only appear in the label name and not in the shortname.

niazim3 commented 6 years ago

Is that the case for chunk types like Theoretical Models, Instance Models, General Definitions, and Data Definitions as well?

Stable:

image

Generated:

image I'm asking because I personally feel that the stable version gives the reader some context of the type of model or definition that's being referenced. However, if it is not significant, it can be removed.

JacquesCarette commented 6 years ago

Doesn't the generated (in the picture above) have an extra T that isn't there in Stable, and you like that extra context? For the 4 types you list, it is reasonable that the shortname include something like T:. I would prefer a separator of some kind, rather than pure concatenation.

niazim3 commented 6 years ago

Alright, is a space after the colon and before the shortname better than pure concatenation?

JacquesCarette commented 6 years ago

Yes.

smiths commented 6 years ago

I like the prefix idea (T: etc).

elwazana commented 6 years ago

@smiths @JacquesCarette For the displayed link what is preferred R: ~~Req~~ or FR / NFR: ~~Req~~, with the second having specific prefixes for Functional/Non-functional requirements?

smiths commented 6 years ago

I like FR/NFR a little better, because I've frequently observed problems with people being confused on the difference. One more opportunity to reinforce this would be nice. Having said that, I don't feel that strongly. If it is R for everything right now, we can always change it to something else in the future. Most of the examples do not currently have very many NFRs.

JacquesCarette commented 6 years ago

I'm one of those people who rather dislikes the (IMHO) unimportant difference being made between FR and NFR. But on this particular issue, I'd let @smiths win.

smiths commented 6 years ago

Thanks @JacquesCarette 😄

elwazana commented 6 years ago

@JacquesCarette @smiths With the implementation of ConceptInstance to create new requirements: https://github.com/JacquesCarette/Drasil/blob/52ce04abb429590e4826627de25645296ad002e1/code/drasil-example/Drasil/NoPCM/Body.hs#L610-L645

There doesn't seem to be a way to distinguish Functional requirements from Non-functional requirements: https://github.com/JacquesCarette/Drasil/blob/52ce04abb429590e4826627de25645296ad002e1/code/drasil-lang/Language/Drasil/Chunk/Concept/Core.hs#L50-L51

Now makeRef has no way to tell which should get the FR: prefix or the NFR: prefix, meaning all requirements will have the R: prefix like they used to before. Should I add a ReqType to ConceptInstance or because this is an unimportant difference, as mentioned above, should I just keep the R: prefix?

Mornix commented 6 years ago

ConceptInstance will (in the near future) provide a prefix to ShortNames through the domain (ConceptDomain) specified, but it will require a little bit of design work. In the case of nr*, the domain is funcReqDom for Functional Requirements. You are correct that there is no way at this moment to retrieve the "value" of the domain from a ConceptInstance.

Adding a ReqType field to ConceptInstance doesn't seem like the correct choice here as ConceptInstance has a more generic purpose than just requirements.

elwazana commented 6 years ago

Ok, then what should the prefix for ConceptInstance be, right now: image (Note: the note that says the rType isn't used is incorrect)

It uses the same prefix as InstanceModel which causes this: image This is definitely wrong considering that Input-Initial-Values is a requirement. Should I make them have no prefix? @JacquesCarette @smiths any thoughts?

smiths commented 6 years ago

I like prefixes, but incorrect prefixes are worse than no prefix. As far as distinguishing FR and NFR, I was under the impression this was easy to implement. It isn't worth a big effort right now. We can use R for everything. Please create an issue though (with the low priority/future enhancement labels) so that we don't lose track of this idea. You can include a reference to this issue.

I would be okay with no prefixes for now, but I worry that there is a design issue that is preventing us from knowing the appropriate prefix. @JacquesCarette and @szymczdm are in a better position that me to judge this.

niazim3 commented 6 years ago

Generated HTML:

image

Stable HTML:

image

@smiths @JacquesCarette Is there a preference for one or the other? It would help me determine whether the shortname for a chunk's label should have the acronym appended at the referencing or creation stage.

JacquesCarette commented 6 years ago

No prefix is definitely better than wrong prefix.

The work that @Mornix is doing will help us have the 'right' information at the right time to be able to adjust the prefixes.

Hmm, I must admit that I don't like seeing the A: prefix in the Assumptions table -- since it is quite clear that it is an assumption. Having that prefix when it is referenced, ah, that is different! But I would not worry about this too much for now.

smiths commented 6 years ago

I like the A: in the generated list of assumptions. It is a matter of taste, but I like the reminder that these are assumptions. It gives the list a nice parallel feel, because everything starts the same. The A:s are like bullets in a bulleted list. This is the kind of thing that it would be nice to eventually be able to customize depending on the user's preferences.

JacquesCarette commented 6 years ago

Indeed, our local disagreement on this display emphasizes that it should be configurable. As far as what to implement now, I would go with whatever 'fits' the current code.

JacquesCarette commented 5 years ago

At some point, this ended up getting fixed. There is no more work that needs done here.