Skip to content
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

Update scale.py docstrings. #10

Merged
merged 6 commits into from
Apr 23, 2021
Merged

Update scale.py docstrings. #10

merged 6 commits into from
Apr 23, 2021

Conversation

EArkhipova-HORIS
Copy link
Collaborator

Update scale.py docstrings.

Copy link
Owner

@ASmirnov-HORIS ASmirnov-HORIS left a comment

Choose a reason for hiding this comment

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

And what about using na_value and guide (not with "none" value) parameters at least in one example?

from lets_plot import *
LetsPlot.setup_html()
x = np.arange(10)
c = np.where(x < 5, '0', '1')
Copy link
Owner

Choose a reason for hiding this comment

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

When you use discrete string variable, maybe it is better to prefer values like 'a' and 'b' or 'x' and 'y' (or whatever symbols, but not digits).

Name of built-in transformation. ('identity', 'log10', 'sqrt', 'reverse')
format : string
trans : str
Name of built-in transformation ('identity', 'log10', 'sqrt', 'reverse').
Copy link
Owner

Choose a reason for hiding this comment

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

If it is all possible values, we could use the following construction:

...
trans : {'identity', 'log10', 'sqrt', 'reverse'}
    Name of built-in transformation.
...

@@ -117,26 +123,25 @@ def scale_x_continuous(name=None, breaks=None, labels=None, limits=None, expand=

Copy link
Owner

Choose a reason for hiding this comment

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

In other places we used the following:

For more info see the `formatting reference <https://jetbrains.github.io/lets-plot-docs/pages/features/formats.html>`_.

from lets_plot import *
LetsPlot.setup_html()
np.random.seed(42)
n = 100
Copy link
Owner

Choose a reason for hiding this comment

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

This variable used only once. You could reject it.

from lets_plot import *
LetsPlot.setup_html()
np.random.seed(42)
n = 1000
Copy link
Owner

Choose a reason for hiding this comment

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

Again, you could drop this extra variable.

LetsPlot.setup_html()
from datetime import datetime
economics_url = 'https://vincentarelbundock.github.io/Rdatasets/csv/ggplot2/economics.csv'
economics = pd.read_csv(economics_url)
Copy link
Owner

Choose a reason for hiding this comment

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

We prefer don't use data that comes through the Internet. At least it should be another example that works offline.

economics = pd.read_csv(economics_url)
economics['date'] = pd.to_datetime(economics['date'])
start = datetime(2000, 1, 1)
economics = economics.loc[economics['date'] >= start]
Copy link
Owner

Choose a reason for hiding this comment

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

This line added because of issue #346. Example should be updated later after fixing the error or right now for preventing this extra moves.


Note
-----
Continuous position scales.
"""
Copy link
Owner

Choose a reason for hiding this comment

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

And where is an example?

data['y'] = np.append(np.random.normal(0, 1, 1000), np.random.normal(3, 1, 500))
ggplot(data, aes('x', 'y')) + geom_point(aes(alpha='..density..'),\\
stat='density2d', contour=False, n=30) + \\
scale_alpha(range=[0.05, 0.9])
Copy link
Owner

Choose a reason for hiding this comment

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

Without loss of usefulness it may be simplified up to

import numpy as np
from lets_plot import *
LetsPlot.setup_html()
np.random.seed(100)
x = np.random.normal(0, 1, 1000)
y = np.random.normal(0, 1, 1000)
ggplot({'x': x, 'y': y}, aes('x', 'y')) + \\
    geom_point(aes(alpha='..density..'), stat='density2d', contour=False, n=30) + \\
    scale_alpha(range=[.01, .99])

from lets_plot import *
LetsPlot.setup_html()
np.random.seed(100)
N = 50
Copy link
Owner

Choose a reason for hiding this comment

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

n = 50

@ASmirnov-HORIS ASmirnov-HORIS merged commit ff5917f into docstring Apr 23, 2021
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