ladybug-tools / ladybug-legacy

:beetle: Ladybug is an environmental plugin for Grasshopper.
http://ladybug.tools
Other
194 stars 82 forks source link

Suggestions for Edits to Ladybug Graphic Standards #196

Open chriswmackey opened 8 years ago

chriswmackey commented 8 years ago

Ladybug Community, This issue has been created so that those who want to question or challenge the Ladybug Graphic Standards have a forum to do so. Also, if there is an important standard or guideline that we are forgetting, please also post that here.

Generally, I think a lot of us would agree that specifics of the standards don't matter as much as the fact that we have a consistent way of communicating as a community (ie. driving on the right or left side of the road does not matter as much as everyone driving on the same side of the road). Therefore, to give us a timeline with this, I think that we should aim to have the standards finalized in written format by the end of November and have them implemented on all components for the stable release in January. So here is the chance to speak now on the standards or forever hold your peace.

chriswmackey commented 8 years ago

I will venture a bet that many of us agree with majority of these standards but there are really only a few that we might discuss here. I am going to post some thoughts of @stgeorges here to get the conversation going (numbers refer to the points in the standard):

6) I somehow feel that coloring the whole component in a color different than working one, until the required inputs are supplied, is still a more valid way of saying to the user: "something is missing, do something", then letting it read the white speech balloon and readMe!. Of course, that's the sole purpose of these two, but coloring the component in different color while required inputs are still not supplied is definitively a more powerful way of informing the user that something is not ok. I think I would support the rule of usage of Remark (in cases of required inputs) if it would color the component to a different color (does not have to be orange), other than regular working one. I the end each grasshopper component when pulled down to the canvas uses Warning level, not Remark.

9) I understand the principle of having components which do not generate the geometry turned off. But at the same time I somehow perceive the feature of hiding some of the outputs for geometry generating components, a bit bizarre to be honest. One either shows every generated geometry or he simply hides them all. And sometimes it is not possible to show them all even if component's preview is turned on: some outputs (like "Sunpath shading" component's: quadrantShadingPercents, quadrantACenergyPercents, hoursPositions) require "Text tag" grasshopper component. Their results can not be seen independently. In my opinion, from a perspective of a regular user, it would be much simpler to turn off the whole component which generates geometry. And then use the appropriate parameters to "see" the one he likes. From this principle I derived the counter one - of having non-geometry generating components.

To give my response counter to @stgeorges , here are my original reasons for following these standards:

6) I recognize that the native GH components do not have a consistent method for dealing with colors when inputs are not connected. For example, the addition component remains grey while the multiplication component turns orange and gives a warning: nativegh Still, I do not think the example of GH team should stop us from setting a standard for ourselves. I prefer having the component remain grey when nothing is connected because the fact that the component is not running is not something that is wrong or that I feel the user needs to be warned about. If I connect something incorrect, then I want to know about it but, especially when I get canvases with tons of components on them, it gets distracting if I have orange components that are not actually malfunctioning. The second reason why I prefer to have the components remain grey is that I send out screenshots of components regularly and even publish these screenshots of components in papers like this: comfortcomps or this: figure 6 - shade benefit component So I know that this does not apply to all developers but, if anyone has the aim of publishing images of components, it is probably in your favor to have a default gre color when nothing is connected.

9) @stgeorges , I think that is is just going to be a mess for a large number of our components if we have to display ALL of the geometric outputs (the radiation study, sunlight hours, and view analysis will become almost unreadable without the user's help, for example). This would be a major problem for new users who are just learning how to use the empty geometry parameter components to turn the preview on/off of specific outputs. If you are arguing that we should just not ouput things like test points from these components, then I think you are too severely limiting the post-processing capabilities for advanced users. I appreciate your reasoning but, unless you can come up with a decent way of satisfying both the beginner and advanced user with the preview on/off capability, this seems like a major uphill battle.

-Chris

mostaphaRoudsari commented 8 years ago

Here is one more from Anders.

..., why is it that you guys set your components to always have icons on? When I use ladybug I always go in and set them back to the default component setting :D

I think he is right. We should probably follow the standards and let the user decide between icons and/or text. We can fix this automatically by adding ghenv.Component.IconDisplayMode = ghenv.Component.IconDisplayMode.application to the header.

mostaphaRoudsari commented 8 years ago

@chriswmackey and @stgeorges . Here is my 2 cents:

  1. I agree that to have the component turn orange as soon as you drop it on the canvas is distracting but at the same time there are couple of issues with many of our components staying gray through the whole process. I would say that once user start connecting the inputs and one or some of the required inputs are missing component should turn orange. We can use Grasshoppers built-in functionalities for this to avoid extra work. Here is where '_' will be pretty important. By default GHPython inputs are set to optional but we should be able to change it with a script like this.
for param in ghenv.Component.Params.Input:
    if param.NickName.startswith("_") and not param.NickName.endswith("_"):
        param.Optional = False

I also have a comment about using the remark balloon in general. It overwrites warning/error balloons on the same component. This is very confusing and I have seen it frequently happening with Run Energy Simulation component. Here is an example. I even tried to change the order and it doesn't really help.

image

  1. There are examples in Grasshopper that outputs are set to Hidden regardless of component's preview such as meshRay. Although I agree that in the first try it can be very confusing not to see the geometry while having the preview on for the component, on the long run it's a very helpful strategy to have them off. This saves everyone from extra unnecessary effort for hiding outputs and connecting selected ones to custom preview. At the end of the day, once you know this is the case it's easy to deal with it so I don't see it as a major issue.
chriswmackey commented 8 years ago

@mostaphaRoudsari and @stgeorges ,

A lot of your arguments have won me over to realizing that we should change a bunch of things. I have run into the issue a lot of times where I am teaching some new users how to use the components and they have not connected a required input but the component remains grey so they don't notice the problem. So I agree with your logic, @mostaphaRoudsari that we should have some code to check required inputs once an input is connected but have it remain grey until an input is connected. To propose a new version of this bullet point, I have re-written it be the following:

6) Components will remain grey when dropped on the canvas but, once any input is connected, the component should use Grasshopper's "Warning" capability (orange component) any time that there is a missing required input or incorrect type of input. Ideally, all of these warnings are given either before or within the input-checking function of the component (so all warnings are given before getting to the main function).

You are right that the over-writing of the Warning with the Remark is a big issue. After thinking about it, it does not seem to be worth waiting for the GH team to fix this. Like the native GH components, we should probably just stop being so sensitive and not be afraid to use the red "Error" feature if something really has gone horribly wrong. Therefore, I would agree that all information that is currently given out as a Remark should become either a warning or information that is printed out of the "ReadMe". This also means that some things that are currently warnings should be errors. To propose a new version of this bullet point, I have rewritten it as the following:

7) If a component gets to its main function but fails to run to completion, Grasshopper's "Error" capability should be used (ie. an energy simulation was started but failed because of incorrect inputs). If the component is running to completion and you you only want to give the user information about how the process has run, please use the "readMe!" output (by printing the message) and not any warnings, errors, or remarks. Grasshopper's "Remark" capability should never be used because it over-rides the color of the component in the case of warnings or errors.

Mostapha, I guess Anders right that we should give the user the choice of displaying the icons or not. Accordingly, I have taken this bullet point out of the standards but I think that this means that we should probably do some revising of the component names right before the next stable release since our current names that are 4 words long are not going to fly in a text display mode. I think this also means that I should just get used to working in GH in icon mode.

Finally, I added a clause at the end of the points that will give us all a little liberty and make clear that these rules should not stop anyone from going above and beyond:

10) No one understands best what a specific component should be telling its user better than the developer and those trying to apply it. Therefore, these standards should be seen as a minimum of what is required and any additional warnings, errors, or information that you realize should be given to the user is desirable and encouraged as long as you are not violating the general structure outlined in this document.

I suggest allowing another 2 weeks for people to see the revised standards and agree on them: https://github.com/mostaphaRoudsari/ladybug/blob/master/resources/Ladybug%20Grahpic%20Standards.md So, if you have any disagreements, you should speak now or forever hold your peace. After the 2 weeks, we will begin the process of implementing all of the standards across all of the components so that everything is good to go by the next stable release in January.

-Chris

mostaphaRoudsari commented 8 years ago

@chriswmackey and @stgeorges when I was trying to address this isseu (ttps://github.com/mostaphaRoudsari/Honeybee/issues/418) I found it tricky the component to stay gray once it's dropped on the canvas and start giving warnings as soon as there is one of the required inputs are connected. It's possible but needs some extra check that may not be really necessary. What is your input on that?

chriswmackey commented 8 years ago

@mostaphaRoudsari , With the code that you had put on this thread earlier, it seems like it will only take an extra 10 lines or so to accomplish what we are thinking with remaining grey until something is connected. You can see these 10 lines of code in the template file that I have just updated in resources: https://github.com/mostaphaRoudsari/ladybug/blob/master/resources/Ladybug_Component_Template.py#L111-L130 We know that we need some type of extra check to put in this feature but it seems so short that I don't really see many issue with it. Let me know if your thoughts, -Chris

stgeorges commented 8 years ago

@chriswmackey and @mostaphaRoudsari , I apologize for taking me too long to reply. I am having some personal issues and work to do, and still haven't resolved either of these. I will try to read up all this topic on Sunday.

mostaphaRoudsari commented 8 years ago

@chriswmackey thank you for the feedback. Looks like you really think it's necessary. Well, in that case I think it should be implemented in Honeybee/Ladybug check instead of having the user write it for every single component. Here is the implementation for Honeybee (https://github.com/mostaphaRoudsari/Honeybee/blob/a19e6888feaf42a8c7fd63a7a9aa8a5bc20d2fd1/src/Honeybee_Honeybee.py#L298-L318) which should be called after checking for version compatibility. Here is my suggestion for the template

#Honeybee check.
if not sc.sticky.has_key('honeybee_release') == True:
    print "You should first let Honeybee fly..."
    ghenv.Component.AddRuntimeMessage(w, "You should first let Honeybee fly...")
    return -1
else:
    try:
        if not sc.sticky['honeybee_release'].isCompatible(ghenv.Component): return -1
        if sc.sticky['honeybee_release'].isInputMissing(ghenv.Component): return -1
    except:
        warning = "You need a newer version of Honeybee to use this compoent." + \
        "Use updateHoneybee component to update userObjects.\n" + \
        "If you have already updated userObjects drag Honeybee_Honeybee component " + \
        "into canvas and try again."
        ghenv.Component.AddRuntimeMessage(w, warning)
        return -1
mostaphaRoudsari commented 8 years ago

PS: Check radiance material components as an example for implementation.

image

PSS: @stgeorges no worries. There is no rush for this.

chriswmackey commented 8 years ago

@mostaphaRoudsari , The plan looks good and I agree that a function like this should be living in LB_LB and HB_HB. I have changed the new component template to reference this function and put in a similar function into LB_LB: https://github.com/mostaphaRoudsari/ladybug/commit/50f4a1ea0c5ee8175ef80e27b6fee2c2dd241cb8 -Chris

stgeorges commented 8 years ago

Hi @chriswmackey , @mostaphaRoudsari,

Thank you for the replies. Apologies for taking me too long to reply.

I recognize that the native GH components do not have a consistent method for dealing with colors when inputs are not connected. For example, the addition component remains grey while the multiplication component turns orange and gives a warning.

They do have consistency. Standards sometimes break when they get to the extremes. Here's David explanation on why "Addition" currently does not turn orange when dragged to the canvas. There is definitively a similar explanation for a couple of more which do not comply to this rule. But the fact that there are exceptions to rules, does not mean we need to take the rules down. And this is why the rest majority of Grasshopper default components comply to this rule. Every Grasshopper Primer released, on its very beginning explains the basics of Grasshopper by dealing with its component color coding system, and why components are orange when dragged to the canvas.

Still, I do not think the example of GH team should stop us from setting a standard for ourselves.

I agree that we do not have to follow each GH team standard. But this is an important standard, and it is essential for the way Grasshopper components work. Again, yes, there are exceptions as we saw, but these exceptions do not mean we have to make it tottaly confusing, and reverse this standard.

I prefer having the component remain grey when nothing is connected because the fact that the component is not running is not something that is wrong or that I feel the user needs to be warned about. If I connect something incorrect, then I want to know about it but, especially when I get canvases with tons of components on them, it gets distracting if I have orange components that are not actually malfunctioning.

And this is exactly what the user should be informed about when he drags the component to the canvas: it requires a certain input, and that component does not contribute to the solution until a required input(s) is supplied. It's such a simple, and yet, useful standard.

The second reason why I prefer to have the components remain grey is that I send out screenshots of components regularly and even publish these screenshots of components in papers like this: So I know that this does not apply to all developers but, if anyone has the aim of publishing images of components, it is probably in your favor to have a default gre color when nothing is connected.

You can open each of those and simply add some code (e.g. "print"). That would last a couple of seconds. You do not make screenshots of components each day, and it is far important to condider the impact some feature has on users, than something we do personally.

There are examples in Grasshopper that outputs are set to Hidden regardless of component's preview such as meshRay.

I have just checked "Mesh | Ray" component and its outputs are not hidden. I may be wrong, but to my knowledge no such default grasshopper component experiences this kind of behavior.

I will comply with both of your opinions, and I already implemented them in the newest edits of Photovoltaics components.

I acknowledge that my opinion on point 9) may be problematic. I would still mention that the current solution for point 6): components which have required inputs, not turning orange when dropped to the canvas, is a very bad revision of default Grasshopper method.

mostaphaRoudsari commented 8 years ago

@stgeorges thank you for the post. It was nostalgic to see the old Grasshopper Primer- Very informative at the same time. I'm convinced that the components should turn orange if any of the required input is missing. I agree (and have seen it so many times) that it makes confusion for new users of Grasshopper. Experienced users know enough to handle warnings like this so I would vote for components to turn to orange once they are dropped on the canvas.

Back to number 9 for the same reason (not making users confused) I still think we should keep them hidden and users can make them visible by choice. Just imagine if Ladybug Radiation analysis shows testPts, testMesh, etc at the same time. IMO that would make a mess. You're also right about MehsRay component. I just checked and it shows up!

Again, all these rules are for making it easier for users to use the tool. If you think that showing all the results in your component will make it easier for users to work with the component then I think it will be totally fine as it is mentioned under number 10:

10) No one understands best what a specific component should be telling its user better than the developer and those trying to apply it. Therefore, these standards should be seen as a minimum of what is required and any additional warnings, errors, or information that you realize should be given to the user is desirable and encouraged as long as you are not violating the general structure outlined in this document.

stgeorges commented 8 years ago

Hi @mostaphaRoudsari ,

It's 2am in here, and my brain is getting really slow. Let me see if I understood you correctly:

Did I understand both of these two?

We should wait for @chriswmackey reply. Still if I understood you correctly on upper two, then do not merge the newest Photovoltaics commits, as they might need to be changed to accommodate possible implementations of upper mentioned features. Again, I will wait to see Chris comment.

mostaphaRoudsari commented 8 years ago

Hello @stgeorges! Yes and yes. It's fine to merge the changes as it is. If you have a newer version we can merge that later. I want the new components to be available to users sooner. Graphic standards is secondary and we will make sure to have a consistent graphic style before the next stable release.

chriswmackey commented 8 years ago

@mostapharoudsari and @stgeorges ,

I am glad to see the conversation continuing. While I am still of the persuasion that grey components with no inputs would be more aesthetically pleasing, if David holds the conviction that orange components without inputs are important for the functional use of GH, then I will hold it as well.

@mostapharoudsari , what is the best way to change this functionality. Should we just edit the "isInputMissing" function?

I am also still strongly on the side of hiding less important visual outputs of the components. Forcing ourselves to adhere to showing all outputs would end up with me removing so much additional functionality and freedom that currently exists in the project.

-Chris

stgeorges commented 8 years ago

Hi @chriswmackey ,

I am also still strongly on the side of hiding less important visual outputs of the components. Forcing ourselves to adhere to showing all outputs would end up with me removing so much additional functionality and freedom that currently exists in the project.

I already complied with this issue, and hid some of my outputs.

mostaphaRoudsari commented 8 years ago

cool! looks like we're coming to an agreement. I modified isInputMissing method to work as expected.

I want to ask for two exceptions for icons. I feel ladybug_ladybug and honeybee_honeybee components are much better to always stay as icons. Is that fine or should we keep it consistent?

stgeorges commented 8 years ago

From my part, I am fine with both of them being icons always.

chriswmackey commented 8 years ago

@mostaphaRoudsari , I am fine with leaving LB_LB and HB_HB as icons. I am also really concerned that some of my longer-named components increasing in size because of their names. I know we have one user who has expressed a desire for this but is this a view that is broadly shared by a lot of members of the community? I would at least like to hear whether you two have any convictions on it before we make this change. -Chris

mostaphaRoudsari commented 8 years ago

@chriswmackey for this particular issue I would say that we need to change the name and shorten the nicknames that will show up on the component. Let me know if you want me to go ahead and change all the components to user-preferred icon/text choice with the same script that updates the versions. I think that way we can start to identify the ones with very long names. Also I'm seeing myself to struggle using Ladybug and Honeybee components with text and not the icons but it will be temporary.

devang-chauhan commented 7 years ago

@mostaphaRoudsari @chriswmackey @stgeorges ,

I am writing here today to put forward an idea, I recently had while working on one of the existing Ladybug components. LB + HB is growing rapidly and it is amazing, but I think in the future, we will have more occurrences of people coming together and expanding / buttressing the existing development. In such scenarios, I believe it would be very helpful if the new developer is provided with a set of test cases by the original developer. This idea is inspired by the idea of a ,"unit tests". Something that is already widely used in the software development community. These test cases should be very helpful to the new developers in testing their implementations before they make a pull request. The new developer can also amend the test cases after consultation with the original developer. So on and so forth.

I am sure all of you will have a view on this, so I shared here. If there's enough consensus on this, may be @chriswmackey can amend these developer notes to that effect.

Thank you all for your time, Devang

chriswmackey commented 7 years ago

@devngc ,

This is something that Mostapha and I have been thinking about for a while but it will probably require a ground-up restructuring of the code if we are to be able to run the tests reliably. As such, I think we should include this on the new version of Ladybug + Honeybee under development rather than try to backpedal and get it to work here.

-Chris

devang-chauhan commented 7 years ago

Sure @chriswmackey. I agree.

Thanks, -Devang

chriswmackey commented 6 years ago

A lot of time has passed since this discussion and I think we should leave this issue open in case anyone has any future comments on the standards of the legacy plugins. It seems worthwhile to focus efforts on the standards of the new plugin, though: https://github.com/ladybug-tools/contributing

mostaphaRoudsari commented 6 years ago

Most of the content here will be useful for the new plugins too. I think we should move this info to a wiki page instead of keeping it as an issue.