Skip to content

[MRG] Deleted 'optional' specifier #16210

Closed
fraboeni wants to merge 1 commit intoscikit-learn:masterfrom
fraboeni:doc_kernel_approximation
Closed

[MRG] Deleted 'optional' specifier #16210
fraboeni wants to merge 1 commit intoscikit-learn:masterfrom
fraboeni:doc_kernel_approximation

Conversation

@fraboeni
Copy link
Copy Markdown
Contributor

with @lopusz and @magda-zielinska

Reference Issues/PRs

#15761

What does this implement/fix? Explain your changes.

Deletes optional specifier and changes array to array-like

Copy link
Copy Markdown
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

thanks for the PR, a few comments

Returns
-------
X_new : {array, sparse matrix}, \
X_new : {array-like, sparse matrix}, \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
X_new : {array-like, sparse matrix}, \
X_new : {ndarray, sparse matrix}, \

Degree of the polynomial kernel. Ignored by other kernels.

kernel_params : mapping of string to any, optional
kernel_params : mapping of string to any
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
kernel_params : mapping of string to any
kernel_params : dict, default=None

Attributes
----------
components_ : array, shape (n_components, n_features)
components_ : array-like of shape (n_components, n_features)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be ndarray here and below

Returns
-------
X_transformed : array, shape=(n_samples, n_components)
X_transformed : array-like of shape (n_samples, n_components)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ndarray

sample_steps : int
Gives the number of (complex) sampling points.
sample_interval : float, optional
sample_interval : float
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please specify the default

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
sample_interval : float
sample_interval : float, default=None

Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

A couple of more comments

Equals the dimensionality of the computed feature space.

random_state : int, RandomState instance or None, optional (default=None)
random_state : int, RandomState instance or None, default=None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
random_state : int, RandomState instance or None, default=None
random_state : int, RandomState instance, default=None

Equals the dimensionality of the computed feature space.

random_state : int, RandomState instance or None, optional (default=None)
random_state : int, RandomState instance or None, default=None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
random_state : int, RandomState instance or None, default=None
random_state : int, RandomState instance, default=None

sample_steps : int
Gives the number of (complex) sampling points.
sample_interval : float, optional
sample_interval : float
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
sample_interval : float
sample_interval : float, default=None

Parameters
----------
sample_steps : int, optional
sample_steps : int
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
sample_steps : int
sample_steps : int, default=2

How many data points will be used to construct the mapping.

random_state : int, RandomState instance or None, optional (default=None)
random_state : int, RandomState instance or None, default=None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
random_state : int, RandomState instance or None, default=None
random_state : int, RandomState instance, default=None

@rth rth added the Needs work label Feb 5, 2020
@noatamir
Copy link
Copy Markdown
Contributor

@fraboeni can you continue to work on this?

@fraboeni
Copy link
Copy Markdown
Contributor Author

@fraboeni can you continue to work on this?

Unfortunately, I am so busy at work at the moment, that I don't find the time.

@fraboeni fraboeni closed this Feb 26, 2020
@fraboeni fraboeni reopened this Feb 26, 2020
@noatamir
Copy link
Copy Markdown
Contributor

@fraboeni thanks for letting us know.
@lopusz and @magda-zielinska, can one of you keep working on it or should I mark it as stalled so others know they can work on it?

@Mariam-ke
Copy link
Copy Markdown
Contributor

Take

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Jun 29, 2020

Closed by #17473.

@cmarmo cmarmo closed this Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants