Skip to content

AxisItem: Account for empty strings in the visibility of text and units#1367

Merged
j9ac9k merged 5 commits intopyqtgraph:masterfrom
dgoeries:axis-item-show-enhance
Oct 15, 2020
Merged

AxisItem: Account for empty strings in the visibility of text and units#1367
j9ac9k merged 5 commits intopyqtgraph:masterfrom
dgoeries:axis-item-show-enhance

Conversation

@dgoeries
Copy link
Copy Markdown
Contributor

  • Account for empty text and units
  • Always reset the visibility of the label, in case of swap

@dgoeries dgoeries changed the base branch from master to develop September 14, 2020 16:19
@j9ac9k j9ac9k changed the base branch from develop to master September 17, 2020 16:52
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Sep 17, 2020

Changed the base, I really should update the CONTRIBUTING.md instructions...

@dgoeries
Copy link
Copy Markdown
Contributor Author

Thanks! I believe I have to look at the tests. An exporter is failing now ...

@dgoeries dgoeries force-pushed the axis-item-show-enhance branch from 768d1e6 to c9aa7ee Compare September 24, 2020 14:53
@dgoeries
Copy link
Copy Markdown
Contributor Author

All tests pass through now. I had to cleanup a bit the AxisItem and disentangle the setLabel from internal use. There is way more potential for separation and cleanup.

There might be a slight regression potential depending on how the method was used. I put this to expected behavior (at least for me, None means empty label.)

Can you have a look again please.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 13, 2020

Thanks for the PR @dgoeries , sorry this took me so long to get to; these diffs look good to me, but given I already hosed up AxisItem once by merging quickly, I'll ask @ixjlyons or @ksunden to take a look.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 15, 2020

After taking another look, I think this is good, thanks again for the PR @dgoeries merging!

@j9ac9k j9ac9k merged commit 3b6eb02 into pyqtgraph:master Oct 15, 2020
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