Skip to content

Consistent "auto*" logic #1282

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.

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

Closed
alexcjohnson opened this issue Jan 4, 2017 · 3 comments
Closed

Consistent "auto*" logic #1282

alexcjohnson opened this issue Jan 4, 2017 · 3 comments
Labels
feature something new

Comments

@alexcjohnson
Copy link
Collaborator

We have a bunch of different auto* attributes scattered about.

  • Some control start/end/size type combinations (autocontour, autobin(x|y) -> {start, end, size}, ncontours/nbins(x|y) is also wrapped up here)
  • Some control just a range pair (autorange -> range:[min, max], heatmap.zauto -> zmin, zmax)
  • At least one other case too (autosize -> width, height).

The only unifying aspect seems to be that the behavior of 2 or 3 output attributes gets set by one auto attribute setting.

Right now they all work fine if you either set auto=true, or if you provide all of the resulting attributes. In #1280 (comment) we started discussing how to handle when you supply only some of the resulting attributes, and the proposal @etpinard and I seem to agree on is:

  • with auto*=true, you ignore whatever was provided for the other attributes, and generate them all fresh. Perhaps the only reason you'd need to do this is if you previously set some non-auto values and you want to toss them out.
  • with auto*=false, you accept whatever was provided for the other attributes, and fill in the rest. So if you want to provide one end of an axis or contour range, or you want to provide the contour size but not the range, you can do that and we'll figure out the rest.

And one extra piece that I think is true now but just for completeness:

  • In all of these situations we should copy the calculated values back to the input traces/layout. This is debatable, I guess, but it's important in practice right now because there are code paths (like restyle/relayout) where you can redo supplyDefaults without calling calc, which is nice because some of these auto calculations are expensive.
@rreusser
Copy link
Contributor

rreusser commented Jan 4, 2017

FWIW, I'm moderately concerned at the moment about the number of small details like this that I won't get right in something as big as carpet plots. My project doesn't really touch other pieces of plotly.js so it's not the worst, but I'm concerned about being unable to fix these small behaviors and caveats I've missed once the original version is released and backward-compatibility can never be broken.

I suspect this concern might be vague enough not to be helpful/useful, but my interest: is there anything that can be done regarding these general philosophical principles in the interest of better consistency and ease of contributing across plotly.js?

Edit: I've been intending to just start documenting random things I've discovered as I work through parts of plotly; I just haven't really gotten my act together and done it.

@alexcjohnson
Copy link
Collaborator Author

@rreusser I think the key is to make sure all this logic as far as possible is in supplyDefaults, with the occasional things like auto* results calculated in a calc step and pointed out in comments and/or PR notes. Then it's just on me and @etpinard to review those parts extra carefully to ensure consistency. I'm not going to pick through every line of your drawing code as long as there are tests that it does what it's supposed to, but the default inheritance I am, for exactly this reason.

@gvwilson
Copy link
Contributor

gvwilson commented Jun 6, 2024

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

@gvwilson gvwilson closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

No branches or pull requests

3 participants