Skip to content

Fix #2798 - Show Uneditable measure name in addition to Display Name#3438

Closed
jmarrec wants to merge 2 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_2798_UneditableMeasureName
Closed

Fix #2798 - Show Uneditable measure name in addition to Display Name#3438
jmarrec wants to merge 2 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_2798_UneditableMeasureName

Conversation

@jmarrec
Copy link
Copy Markdown
Collaborator

@jmarrec jmarrec commented Mar 4, 2019

Fix #2798 - Show Uneditable measure name in addition to Display Name

Upon dragging a new measure:

  • name goes to "Original Name" and cannot be modified
  • display_name will serve as default for the (Editable) 'Name' in the Edit Pane (if same display name is found, then add an integer counter to display name). Whatever is set there will be shown in the Measure List in the main pane, and will be saved in workflow.osw as well.

rationale

@jmarrec jmarrec requested a review from macumber March 4, 2019 15:13
@jmarrec
Copy link
Copy Markdown
Collaborator Author

jmarrec commented Mar 4, 2019

@DavidGoldwasser Let me know if that solves your issue, and whether you'd like to request a different terminology for 'Original Name' if need be.

@macumber
Copy link
Copy Markdown
Contributor

macumber commented Mar 4, 2019

There are actually three names here:

Measure machine name - name from measure.xml
Measure human readable name - display_name from measure.xml
Workflow step human readable name - name from workflow step in OSW (https://github.com/NREL/OpenStudio-workflow-gem/blob/develop/spec/schema/osw.json#L345)

There are also description and modeler description fields in the OSW. I think that the center pane should show information from the OSW. The right pane should show information from the measure.xml. Is that what your change is doing?

@DavidGoldwasser
Copy link
Copy Markdown
Collaborator

@macumber I don't think the "Name" field in the "Edit" tab can come from the measure.xml, seems like it has to be from OSW steps so that multiple instances of the same measure can have unique name. the non editable "Original Name" could come from the measure.xml

@jmarrec
Copy link
Copy Markdown
Collaborator Author

jmarrec commented Mar 4, 2019

If I follow correctly I think I got it right:

  • if you drag from library, you pull info from measure.xml to create a new workflow step. The original name is created from measure.xml tag 'name' (uneditable), the Editable name is generated from tag 'display_name' (with an integer if already exists among the workflow steps that are already there)
  • in the edit pane, what you change is the workflow step (inside workflow.osw in companion directory), which stores the original name in 'name' (won't change since not editable, but you have to be able to reload it don't you?) And the one you can edit in 'display_name'

Basically measure.xml is used only when dragging from library to create a workflow step. It was already the case really, except it was using the display_name tag from measure.xml to initialize the 'name' tag in workflow step (and display_name in workflow step didn't exist).

This appears to work I think. I'm not sure if I'm clarifying here or adding confusion (I'm on my phone sorry...)

I can take a gif of it in action tonorrow or you can build my branch and see yourself (no Idd change, should rebuild fast)

@macumber
Copy link
Copy Markdown
Contributor

macumber commented Mar 5, 2019

Yeah I think it is confusing. I lobbied for a more literal display on this tab but got shot down., I would prefer to have the following:

"Measure Step Name" - editable, initialized from measure.xml display name, writes to OSW measure step name
"Measure Step Description" - editable, initialized from measure.xml description, writes to OSW measure step description
--- Section Break --- (collapsible section, initially collapsed)
"Measure Name" - read only, from measure.xml
"Measure Directory" - read only, from measure.xml
"Measure Display Name" - read only, from measure.xml
"Measure Class Name" - read only, from measure.xml
"Measure Description" - read only, from measure.xml
"Measure Modeler Description" - read only, from measure.xml
--- Section Break ---
Inputs

@macumber
Copy link
Copy Markdown
Contributor

macumber commented Mar 5, 2019

As far as I know, nothing in the app (except for creating measures) will alter the measure.rb file. The measure.rb file is the source for measure name, display name, class name, description, and modeler description when check for updates runs

@jmarrec
Copy link
Copy Markdown
Collaborator Author

jmarrec commented Mar 5, 2019

Ok. So whenever you clicked on a measure step, the edit pane gets brought up. Currently information is all coming from the workflow.osw,.

With your proposed scenario, do you:

  • Store all of a that uneditable stuff from measure.xml into the workflow.osw when you drag, or,
  • Read the measure.xml every time? (Or even query the measure.rb via measure manager?)

I find your proposed scenario to be sensible, I'm just wondering if this extra work is worth it in comparison of the (IMHO) little gain in functionality, and I'm afraid of the added complexity if you need to re-read measure.xml every time (potential path issues, extra operations to be done).

@macumber
Copy link
Copy Markdown
Contributor

macumber commented Mar 5, 2019

I feel that the term "Original Measure Name" is confusing things even more. If this edit pane is restricted to items in the OSW only then there is no "Original Measure Name" in the OSW, the closest thing would be "Measure Dir Name". IMHO "Measure Dir Name" is useful, is not user editable, and is what WorkflowJSON actually uses to locate the measure.

For now, let's constrain the UI to only show content in the OSW and keep the keys aligned with what is in the OSW:

"Name" - editable step name
"Measure Dir Name" - non-editable measure dir name <-- new field
"Description" - editable step description
"Modeler Description" - non-editable step modeler description

@jmarrec
Copy link
Copy Markdown
Collaborator Author

jmarrec commented Mar 5, 2019

The terminology could be changed to Name and display Name to match the workflow OSW tag if that's the problem.

So, bottom line, I added "display_name" to OSW, should I just undo all of that work and just use the measure_dir_name then?

@macumber
Copy link
Copy Markdown
Contributor

macumber commented Mar 5, 2019

Yes please, no changes should be made to the OSW. Let's restart and just add "Measure Dir Name", the "Original Name" is a red herring and we don't want to pursue it.

@macumber macumber added this to the Version 2.8.0 milestone Mar 20, 2019
Copy link
Copy Markdown
Contributor

@macumber macumber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to constrain ourselves to existing keys in the OSW schema:

https://github.com/NREL/OpenStudio-workflow-gem/blob/develop/spec/schema/osw.json#L324

in OSW name is the editable name of the MeasureStep, measure_dir_name is the uneditable name of the measure directory.

@jmarrec jmarrec closed this Mar 21, 2019
@jmarrec jmarrec deleted the PR_opened/Fix_2798_UneditableMeasureName branch March 21, 2019 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants