Skip to content

feat: Add {x,y}gap parameter to heatmaps#385

Merged
andrei-ng merged 3 commits intoplotly:mainfrom
atticus-sullivan:feat-heatmap-gaps
Feb 12, 2026
Merged

feat: Add {x,y}gap parameter to heatmaps#385
andrei-ng merged 3 commits intoplotly:mainfrom
atticus-sullivan:feat-heatmap-gaps

Conversation

@atticus-sullivan
Copy link
Contributor

see #383 (comment) basically

The docs say there are parameters {x,y}gap for heatmap-traces. This PR adds these parameters.

See the docs:

In JS they refer to the type as "number greater than or equal to 0" similar in the python docs. By experimenting with the JS version (plotly-3.3.1.min.js) I can say that (in that version) you can also pass floats and strings. Numbers < 0 seem to be simply "truncated" to 0.

Thus I chose to use the NumOrString type, but I'm not 100% on whether that's correct (especially regarding negative numbers).

.text(vec!["te", "xt"])
.transpose(true)
.visible(Visible::LegendOnly)
.x_axis("x")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@atticus-sullivan For completeness, could you extend the test with these two setters and the expected json.
With that in place I will merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense done 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense done 👍

@atticus-sullivan could you also update the expected value. You need to update the json with the two values for xgap and ygap

Next time you can also give rights on your fork to maintainers so maintainers can push fixes directly to your fork.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry my bad, I was really too fast on this. Now it's fixed.

You should have the right to make changes to the branch (at least the checkbox is ticked)

@atticus-sullivan
Copy link
Contributor Author

I feel almost bad asking 🙈 do you have a "release cycle"? (basically when approximately will this be part of a new release)

But I don't want to pressure (especially with that other bug/thing still open ^^), I'm just curious

@andrei-ng
Copy link
Collaborator

I feel almost bad asking 🙈 do you have a "release cycle"? (basically when approximately will this be part of a new release)

But I don't want to pressure (especially with that other bug/thing still open ^^), I'm just curious

No, not a release cycle, not really. Never feel bad for asking, it is a valid question.

I can make a patch release after we merge this.

@atticus-sullivan
Copy link
Contributor Author

That would be very kind (but not necessary if the other bugfix follows soon, no need to create release after release).

Should I squash the commits into one before you merge, or are you fine with the current commits?

@andrei-ng
Copy link
Collaborator

andrei-ng commented Feb 12, 2026

Should I squash the commits into one before you merge, or are you fine with the current commits?

@atticus-sullivan That would be nice.

@andrei-ng andrei-ng merged commit 8a8dab9 into plotly:main Feb 12, 2026
30 of 32 checks passed
@andrei-ng
Copy link
Collaborator

Should I squash the commits into one before you merge, or are you fine with the current commits?

@atticus-sullivan That would be nice.

I did it from the web UI. Thanks for the PR fix

@atticus-sullivan
Copy link
Contributor Author

No, not a release cycle, not really. Never feel bad for asking, it is a valid question.

I can make a patch release after we merge this.

With #387 and this (#385) merged, can you make a patch/minor release ready? (it's not totally urgent though)

@andrei-ng
Copy link
Collaborator

I just made one. Try it out and let me know.

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