Skip to content

fix(style): remove global styles that interfere with structure plots (FXC-1802)#2659

Merged
frederikschubertflex merged 3 commits into
developfrom
frederikschubertflex/minimal-automatic-styles
Jul 16, 2025
Merged

fix(style): remove global styles that interfere with structure plots (FXC-1802)#2659
frederikschubertflex merged 3 commits into
developfrom
frederikschubertflex/minimal-automatic-styles

Conversation

@frederikschubertflex
Copy link
Copy Markdown
Contributor

@frederikschubertflex frederikschubertflex commented Jul 15, 2025

This PR reverts some of the automatic global styles that were added in #2428 due to visualisation issues in structure plots.

Greptile Summary

This PR makes a focused change to the global matplotlib style settings in tidy3d/style.mplstyle to fix visualization issues with structure plots. The changes remove automatic grid lines and custom edge colors that were causing interference with structure plot rendering, while maintaining the color cycle definition.

Specifically, the PR:

  • Removes the automatic grid lines setting (axes.grid: True and grid.linestyle: :)
  • Removes the custom edge color setting (axes.edgecolor: "#ECEBEA")
  • Preserves the color cycle definition for consistent plot coloring

This change reverts part of PR #2428 where these global styles were initially added, as they were found to cause unintended effects on structure plot visualizations.

Confidence score: 5/5

  1. This PR is very safe to merge as it only removes interfering style settings without affecting core functionality.
  2. The changes are minimal, well-documented, and directly address a specific visualization issue.
  3. Focus areas: tidy3d/style.mplstyle - verify that removing these style settings doesn't negatively impact other visualization use cases.

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

@frederikschubertflex frederikschubertflex self-assigned this Jul 15, 2025
@frederikschubertflex frederikschubertflex added the 2.9 will go into version 2.9.* label Jul 15, 2025
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Reviewing changes made in this pull request

Copy link
Copy Markdown
Collaborator

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

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

Change looks fine but there seems to be a failing test?

@frederikschubertflex
Copy link
Copy Markdown
Contributor Author

Fixed the test! So after the suite has finished this is good to go.

@github-actions
Copy link
Copy Markdown
Contributor

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@frederikschubertflex frederikschubertflex added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 15, 2025
@frederikschubertflex frederikschubertflex added this pull request to the merge queue Jul 15, 2025
@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Jul 15, 2025
@daquinteroflex daquinteroflex enabled auto-merge July 15, 2025 16:45
@daquinteroflex daquinteroflex added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 15, 2025
@frederikschubertflex frederikschubertflex added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 15, 2025
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Jul 16, 2025
@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Jul 16, 2025
@daquinteroflex daquinteroflex added this pull request to the merge queue Jul 16, 2025
@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Jul 16, 2025
@frederikschubertflex frederikschubertflex added this pull request to the merge queue Jul 16, 2025
Merged via the queue into develop with commit 7e2bcda Jul 16, 2025
24 checks passed
@frederikschubertflex frederikschubertflex deleted the frederikschubertflex/minimal-automatic-styles branch July 16, 2025 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.9 will go into version 2.9.*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants