Skip to content

WIP: Multiple y-axis formatting #6320

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
wants to merge 10 commits into from
Closed

WIP: Multiple y-axis formatting #6320

wants to merge 10 commits into from

Conversation

hannahker
Copy link
Contributor

@hannahker hannahker commented Sep 20, 2022

A simpler starting implementation of #6319, where we assume a constant offset (xshift) applied to anchored y axes that may be set to either the left or right side of a plot.

To-do's:

  • Offset axes based on width of previously drawn axes on the same side
  • Remove need for automargin=True to be set for the first y-axes. This is causing multiple redraws of the axes, but without it the GetLinePosition() function gets called before the axis is drawn and so there isn't any xshift to be applied.
  • Address the Error: <text> attribute x: Expected length, "NaN" errors (although I can't find any impact of this on the output plot.
  • Fix all the failing tests :(

anchorAxis = {
_offset: ax._anchorAxis._offset - xshift,
_length: ax._anchorAxis._length
};
Copy link
Contributor

Choose a reason for hiding this comment

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

ax._anchorAxis contains several other keys and values that may be of interest for other functions.

Wondering it might be better to use Lib.extendFlat e.g.

            anchorAxis = extendFlat({}, ax._anchorAxis, {
                _offset: ax._anchorAxis._offset - xshift,
                _length: ax._anchorAxis._length
            });

or alternatively mutate ax._anchorAxis directly i.e.

            ax._anchorAxis._offset = ax._anchorAxis._offset - xshift;
            ax._anchorAxis._length = ax._anchorAxis._length;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into difficulties with the latter in that this function is called multiple times during the axis drawing so the offset value ends up as double what it should be.

Could you clarify the issue with the modification here? I'm not totally understanding as this seems to be following the same pattern as was already implemented eg. in lines 3809-3817 below.

@hannahker hannahker mentioned this pull request Sep 20, 2022
@hannahker hannahker marked this pull request as ready for review September 22, 2022 18:50
@@ -2345,12 +2344,20 @@ axes.drawOne = function(gd, ax, opts, allDepths) {
// to determine how much to shift this one out by
// TODO: Also need to account for the expected depth of the current axis
// (if drawing from the left inwards)
if(axLetter === 'y') {
if(axLetter === 'y' & !ax.position > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if understand the statement here.
!ax.position would be true or false and we are comparing it to be greater than zero?
Let's rewrite this statement.

@hannahker
Copy link
Contributor Author

Closing this in favour of #6334

@hannahker hannahker closed this Oct 4, 2022
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.

2 participants