fix: Dynamically compute default zoom/center for map traces

[camdecoster]

Description

Use lat/lon points to dynamically compute the default zoom/center for scattermap and densitymap traces.

Closes #7674.

Changes

• Add check during handleDefaults to dynamically computer map bounds
• Add tests
• Linting/formatting

Screenshots

Before	After

Testing

• Be on master
• Run the following mock through Plotly devtools

  {
    "data": [
      {
        "hovertext": ["San Marino", "Cairo", "Istanbul", "Trondheim"],
        "lat": [43.9360958, 30.06263, 41.01384, 63.43049],
        "lon": [12.4417702, 31.24967, 28.94966, 10.39506],
        "marker": { "color": "#f00" },
        "mode": "markers",
        "type": "scattermap"
      }
    ],
    "layout": { "width": 900, "height": 600 }
  }

• Note that the Trondheim point is outside of the frame
• Switch to this branch
• Run the same mock
• Note that the scatter points are nicely framed
• Pan/zoom around
• Click the reset view button
• Note that the view is still nicely framed

Notes

• This PR is a combination of work done by me, @palmerusaf (Dynamic map defaults #7692), and @DhruvGarg111 (feat: Add symlog scale, scattermap zoom fix, and line dash support #7718)
• This is landing in v4.0, so it will be on by default. Users that don't want auto-framing should specify a center and/or zoom.
• The first commit handles linting/formatting for a couple of files. That commit can be ignored during review.

[emilykl]

I think you've addressed all my previous comments.

I did find one bug though: Plotly functions which update only the data don't adjust the auto-fit bounds like they (probably?) should.

Here's a sample mock with 2 traces:

{
  "data": [
    {
      "hovertext": ["Montreal", "New York", "Los Angeles", "San Francisco"],
      "lat": [45.5017, 40.7128, 34.0522, 37.7749],
      "lon": [-73.5673, -74.006, -118.2437, -122.4194],
      "marker": { "color": "#0f0" },
      "mode": "markers",
      "type": "scattermap"
    },
    {
      "hovertext": ["San Marino", "Cairo", "Istanbul", "Trondheim"],
      "lat": [43.9360958, 30.06263, 41.01384, 63.43049],
      "lon": [12.4417702, 31.24967, 28.94966, 10.39506],
      "marker": { "color": "#f00" },
      "mode": "markers",
      "type": "scattermap"
    }
  ],
  "layout": {
    "width": 900,
    "height": 600
  }
}

Calling Plotly.restyle(gd, {"lat": [[45.5017]]}, [0]) removes the latter 3 cities from the first trace, but doesn't update the auto-bounds.

Calling Plotly.deleteTraces(gd, 1) deletes the second trace, but doesn't update the auto-bounds.

On the other hand, calling

Plotly.react(gd, {
      "hovertext": ["Montreal", "New York", "Los Angeles", "San Francisco"],
      "lat": [45.5017, 40.7128, 34.0522, 37.7749],
      "lon": [-73.5673, -74.006, -118.2437, -122.4194],
      "marker": { "color": "#0f0" },
      "mode": "markers",
      "type": "scattermap"
    })

works as expected (updates data, and updates auto-bounds, which makes sense because no center/zoom settings are passed in the layout).

I guess you could make an argument that this is expected behavior (functions which only update data should not affect the layout) but I'm not sure it's what we want. Do we have any other layout values which determine their defaults based on values in the data? If so, what do we do in those cases? I'll sleep on it.

Relatedly, could you add an end-to-end Jasmine test which creates a new scattermap plot and then checks the zoom and center of the resulting map object? And several more which then call the various Plotly functions on that plot, and check that the resulting zoom and center match whatever we decide the correct behavior to be?

Finally, I almost hate to bring it up, but... it's probably worth testing whether the auto-bounds behave as expected with respect to layout.uirevision.

[DhruvGarg111]

Dug into the restyle/deleteTraces behavior. The auto-fit only runs in handleDefaults, and only when center and zoom are both undefined. The problem is that once the map renders, saveViewToLayout writes the computed center/zoom back onto _input (so pan/zoom and reset-view stick around). After that first render, they're no longer undefined, so the next supplyDefaults from restyle/deleteTraces skips the entire fit-bounds block. react works because you're handing it a fresh layout with center/zoom undefined again.

Re: your question about other layout values that key off the data, autorange is the obvious comparison. It's the same idea, but it doesn't get stuck like this because it keeps an explicit autorange flag instead of overwriting the user's input with the computed value. That's basically what's missing here: "auto" and the saved-view state are stored in the same fields, so auto-fit can only fire once.

If we do want restyle/deleteTraces to re-fit, copying the autorange approach seems like the right path: keep a flag for "user never set center/zoom," recompute on data changes, and let uirevision decide whether to preserve a manual pan/zoom or throw it away. The map.fitBounds re-fit path already exists, so it's mostly about fixing the gating.

Since this is going into v4.0, do you want the full re-fit behavior in this PR, or should we ship what we have (auto-fit on initial plot + react) and handle the data-only case in a follow-up? Happy to do either and take the implementation.

I'll add the Jasmine tests for center/zoom across the different calls, plus the uirevision case, once we settle on the expected behavior.

thanks.

[palmerusaf]

Sorry for the late reply. I've been quite busy with work and school. One issue I found with this implementation is that it doesn't handle points around the anti-meridian well.

This seems like a common issue. Even one of our dependencies, turf.js. doesn't handle it well either.
One solution that works is to normalize the lat inputs to 360, sort them, iterate over them to find the largest gap, then take the complement of that. However, this introduces sorting which will be expensive when the input is large.

[DhruvGarg111]

@palmerusaf , can you please share the data you used here. i need to reproduce this to find the exact error.
thanks

[camdecoster]

@palmerusaf that case should be handled. Have you tested your example with the latest changes? From everything that I've tried with coords that span the antimeridian, the fit works as it should.

[camdecoster]

@emilykl restyle and deleteTraces should work now. autorange seems to be the closest existing attribute that sizes something based upon the input data. Regardless, I think that auto-fit should do it's job across many functions, not just the initial render. I added a number of tests to cover expected behavior. Regarding uirevision, I think that will need some work but I'd like to handle that in a follow up PR.

[palmerusaf]

@camdecoster Just tested looks good now.

I just tested through the test dashboard using the following mock data.

{
  "data": [
    {
      "hovertext": [
        "P1",
        "P2",
        "P3",
        "P4"
      ],
      "lat": [
        43.9360958,
        30.06263,
        41.01384,
        63.43049
      ],
      "legendgroup": "",
      "lon": [
        170,
        180,
        -180,
        -170
      ],
      "marker": {
        "color": "#636efa"
      },
      "mode": "markers",
      "type": "scattermap"
    }
  ]
}

[camdecoster]

Great. Thanks for checking.

[emilykl]

Can you add a comment here explaining the meanings and usage of _fitBounds vs. _fitView ?

[camdecoster]

Good idea. I added some clarifying language in a couple of places.

[emilykl]

5 decimal places seems like a lot

[emilykl]

Not a blocker, but this comment still isn't super clear to me. I would prefer something like

    // - `_fitBounds` contains the lon/lat coords computed from the data by getMapFitBounds(), and is used for <purpose>
    // - `_fitView` contains the view attributes of the MapLibre map, and is updated in createMap() and updateMap() when <circumstances>

[emilykl]

Looks good! I think there's still a little bit of edge case weirdness around stuff like clicking the "reset view" button after interacting and then calling e.g. Plotly.restyle(), but I don't think that's a blocker for getting this feature out.

One final request: Could you update the info on supplyDefaults in CONTRIBUTING.md to reflect this change? Maybe say something like we try to avoid looping over data arrays in supplyDefaults, but there are a few exceptions.