Skip to content

Changing Histogram parameter descriptions #623

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 10, 2016
Merged

Changing Histogram parameter descriptions #623

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
44568ad
changed descriptions of some histogram parameters
yankev Jun 9, 2016
e259d52
changed wording
yankev Jun 9, 2016
4d60e4f
added periods
yankev Jun 9, 2016
a22ce0d
added joins to fix strings
yankev Jun 9, 2016
293ce37
fix indent issues
yankev Jun 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/traces/histogram/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,21 @@ module.exports = {
role: 'style',
description: [
'Determines whether or not the x axis bin attributes are picked',
'by an algorithm.'
'by an algorithm. Note that this should be set to false if you',
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to my comment in #607, autobinx does not need to be set to false as it is defaulted to false in if xbins.start and xbins.end are valid inputs (see logic).

But we don't really have a way to communicate smart default logic in our descriptions yet.

So, I think this patch is fine for now. But we should starting thinking about a better approach cc @cldougl

Copy link
Member

Choose a reason for hiding this comment

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

@etpinard yep agreed -- I'll try to think of wording to express the logic ( plotly/documentation#428 ). I really want to change that because I think that having 1 less variable to worry about to get a result makes these 'trickier' attributes w/ dependents more approachable for new users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I was thinking about adding another meta filed to our attribute descriptions.

Something like dfltDependsOn:

autobinx: {
  valType: 'boolean',
  dflt: false,
  dfltDependsOn: ['xbins.end', 'xbins.start']
};

Copy link
Member

Choose a reason for hiding this comment

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

I like that a lot -- that's a lot easier for me to read than trying to pull the default info from the description

Copy link
Member

Choose a reason for hiding this comment

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

but we should still have some note that expresses you don't have to set this attribute, we'll logically set it for you with the defaultDepends

Copy link
Contributor

@mdtusz mdtusz Jun 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

What would be really helpful is if we use some delimiter to mark things as linkable attributes so that in the docs, users can click on them and go to their definition. E.g.

description: 'This sets some thing, provided that [context.attribute](toplevel.midlevel.bottomlevel) is true.' would output:

"This sets some thing, provided that context.attribute is true."

I'm not sure what the process of attributes.js files -> docs is, but I'd think we could make it happen.

'want to manually set the number of bins using the attributes in',
'xbins.'
].join(' ')
},
nbinsx: {
valType: 'integer',
min: 0,
dflt: 0,
role: 'style',
description: 'Sets the number of x axis bins.'
description: [
'Specifies the maximum number of desired bins. This value will be used',
'in an algorithm that will decide the optimal bin size such that the',
'histogram best visualizes the distribution of the data.'
].join(' ')
},
xbins: makeBinsAttr('x'),

Expand All @@ -100,15 +106,21 @@ module.exports = {
role: 'style',
description: [
'Determines whether or not the y axis bin attributes are picked',
'by an algorithm.'
'by an algorithm. Note that this should be set to false if you',
'want to manually set the number of bins using the attributes in',
'ybins.'
].join(' ')
},
nbinsy: {
valType: 'integer',
min: 0,
dflt: 0,
role: 'style',
description: 'Sets the number of y axis bins.'
description: [
'Specifies the maximum number of desired bins. This value will be used',
'in an algorithm that will decide the optimal bin size such that the',
'histogram best visualizes the distribution of the data.'
].join(' ')
},
ybins: makeBinsAttr('y'),

Expand Down