Extended plotting and added sampling #667
Conversation
Add parameters for labelling/coloring and naming points of a point cloud; added parameters for custom marker size and opacity; added parameters to toggle the plot being to scale or not, and printed or not.
Add directory "sampling" containing "sample_point_clouds.py". The file contains functions to sample points from a round sphere, a standardly embedded torus and a circle in the x-y-plane in $\mathbb{R}^3$ uniformly and at random.
| import plotly.graph_objs as gobj | ||
|
|
||
| from gtda.utils import validate_params | ||
| from gtda.utils.intervals import Interval as Int |
There was a problem hiding this comment.
is there a good reason to import Interval as Int ? Interval is already a good name and I don't see why you should change it as Int. Moreover, Int is really a bad variable name. It is similarm to int but it is totally another thing.
| marker_size=5, | ||
| opacity=0.8, | ||
| to_scale=False, | ||
| display_plot=False): |
There was a problem hiding this comment.
- indentation is wrong
- typing is missing
| # If no labels provided, just enumerate data points, and record | ||
| # if there were user-provided labels to use in `names`. | ||
| labels_were_provided = labels is not None | ||
| if not labels_were_provided: |
There was a problem hiding this comment.
you can just write
if labels is not None:
labels = np.arange(point_cloud.shape[0])or in one line
labels = np.arange(point_cloud.shape[0]) if labels is not None else labelsThere was a problem hiding this comment.
You can also put np.arange(point_cloud.shape[0]) as default value for labels. I think this is the best solution.
|
|
||
| if names is not None: | ||
| if not labels_were_provided: | ||
| raise ValueError("No lables were provided.") |
There was a problem hiding this comment.
this should be done at the very beginning of the method not after all the calls at the validate_params
| {"display_plot": {"type": (bool,)}}) | ||
|
|
||
| if dimension is None: | ||
| dimension = np.min((3, point_cloud.shape[1])) |
|
|
||
| def sphere_sampling(n=1000, r=1, noise=0): | ||
| """Uniformly and randomly samples points from a round | ||
| sphere centered at the origin of 3-space. |
There was a problem hiding this comment.
from a round sphere? There exists a sphere which is not round? Or are there different types of spheres?
delete round here.
| from gtda.utils.intervals import Interval | ||
|
|
||
|
|
||
| def sphere_sampling(n=1000, r=1, noise=0): |
There was a problem hiding this comment.
you should use descriptive name. For example, r is for radius. Therefore, use radius as variable name.
| for i in range(n): | ||
| theta = np.random.uniform(low=0, high=2*np.pi) | ||
| phi = U_phi_inverse(np.random.uniform()) | ||
| pt = parametrization(theta, phi) |
There was a problem hiding this comment.
you are using the method parametrisation just here in this for loop.
Just write
pt = np.array(
[
r*np.cos(theta)*np.sin(phi),
r*np.sin(theta)*np.sin(phi),
r*np.cos(phi),
]
)it is much easier to understand what it is happening
And use another name for the variable pt. Is it a point? Just use the name point.
| return points | ||
|
|
||
|
|
||
| def torus_sampling(n=1000, R=3, r=1, noise=None): |
There was a problem hiding this comment.
I will suggest to find better names for the variables
There was a problem hiding this comment.
Why are you using these default values here?
| validate_params({"noise": noise}, | ||
| {"noise": {"type": (int, float), | ||
| "in": Interval(0, np.inf, closed="left")}}) | ||
|
|
Reference issues/PRs
Types of changes
Description
Screenshots (if appropriate)
Any other comments?
Checklist
flake8to check my Python changes.pytestto check this on Python tests.