Skip to content

ENH: Expose symlog scaling in plotting API #24871

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 51 commits into from

Conversation

charlesdong1991
Copy link
Member

@WillAyd WillAyd added the Visualization plotting label Jan 21, 2019
@WillAyd
Copy link
Member

WillAyd commented Jan 21, 2019

Not terribly familiar with this code but looks OK to me at a glance.

cc @TomAugspurger - too late to get this in for v0.24 at this point right?

@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #24871 into master will decrease coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24871      +/-   ##
==========================================
- Coverage   92.38%   92.38%   -0.01%     
==========================================
  Files         166      166              
  Lines       52412    52416       +4     
==========================================
+ Hits        48423    48425       +2     
- Misses       3989     3991       +2
Flag Coverage Δ
#multiple 90.81% <33.33%> (-0.01%) ⬇️
#single 42.88% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_core.py 83.59% <33.33%> (-0.17%) ⬇️
pandas/util/testing.py 88.13% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12bb6d0...db4440d. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #24871 into master will decrease coverage by 49.5%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24871       +/-   ##
===========================================
- Coverage   92.38%   42.88%   -49.51%     
===========================================
  Files         166      166               
  Lines       52412    52406        -6     
===========================================
- Hits        48423    22476    -25947     
- Misses       3989    29930    +25941
Flag Coverage Δ
#multiple ?
#single 42.88% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_core.py 17.68% <0%> (-66.08%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.35%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12bb6d0...36c8eb4. Read the comment docs.

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

It's not clear to be that this is working correctly. You should add a test that includes an assertion that ax.get_yscale() == 'symlog'.

What's the user-facing API? I was thinking something like

df.plot(logy="sym")

so that a user can get regular log with log=True, symlog with log="symlog" or "sym", not sure which. And maybe accept "logit" too.

@@ -310,9 +310,15 @@ def _setup_subplots(self):
axes = _flatten(axes)

if self.logx or self.loglog:
[a.set_xscale('log') for a in axes]
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably slightly cleaner to set

scale = 'symlog if self.sym else 'log'
[a.set_xscale(scale) for a in axes]

@mdekstrand
Copy link

I like @TomAugspurger's proposed UI - it's exactly what I had in mind. Since the documented API is already logx=True, allowing logx='sym' to turn on symlog makes a lot of sense to me as an API user.

@charlesdong1991
Copy link
Member Author

Thank you very much @mdekstrand , it's good to know! since my proposal will be quite significant API change, so i would like to hear all of yours opinions first before submitting the pr... i will then keep Tom's proposal first in it!

charlesdong1991 and others added 18 commits January 22, 2019 23:04
…andas-dev#24715)

* DOC: Implementing redirect system, and adding user_guide redirects

* Using relative urls for the redirect

* Validating that no file is overwritten by a redirect

* Adding redirects for getting started and development sections
* Fixed heading on whatnew
* Remove empty scalars.rst
* Revert BUG-24212 fix usage of Index.take in pd.merge

xref pandas-dev#24733
xref pandas-dev#24897

* test 0.23.4 output

* added note about buggy test
…s-dev#24882)

* DOC: Add experimental note to DatetimeArray and TimedeltaArray
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

pls add a whatsnew note in 0.25.0 other enhancements

valid_log = [False, True, 'sym']
for i in [self.logx, self.logy, self.loglog]:
if i not in valid_log:
raise ValueError("Wrong input for log option.")
Copy link
Contributor

Choose a reason for hiding this comment

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

show the option that was passed

@@ -309,10 +309,20 @@ def _setup_subplots(self):

axes = _flatten(axes)

if self.logx or self.loglog:
valid_log = [False, True, 'sym']
for i in [self.logx, self.logy, self.loglog]:
Copy link
Contributor

Choose a reason for hiding this comment

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

you can do this with a set operation; is None also valid?

@charlesdong1991
Copy link
Member Author

sorry, since i have several branches working on the same time... there are some git issues that i didn't notice so i messed it up... i will close this PR, and reopen another one... sorry...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Expose symlog scaling in plotting API