Skip to content

Made required changes to kernel_approximation.py#17473

Merged
thomasjpfan merged 6 commits intoscikit-learn:masterfrom
Mariam-ke:deleted_specifier
Jun 6, 2020
Merged

Made required changes to kernel_approximation.py#17473
thomasjpfan merged 6 commits intoscikit-learn:masterfrom
Mariam-ke:deleted_specifier

Conversation

@Mariam-ke
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

References #16210

What does this implement/fix? Explain your changes.

Updated the required changes as per the suggested changes on the files changed tab

Any other comments?

@NicolasHug
Copy link
Copy Markdown
Member

looks like there's a small issue with the submitted changes @Mariam-ke (2k+ changes)

Might be related to #17341 ?

@Mariam-ke
Copy link
Copy Markdown
Contributor Author

Yup I saw the untracked files and added them as well.

@amueller
Copy link
Copy Markdown
Member

amueller commented Jun 6, 2020

@Mariam-ke can you please remove them? they shouldn't be added.

@Mariam-ke Mariam-ke force-pushed the deleted_specifier branch from b99ad93 to 95c26bc Compare June 6, 2020 18:16
@Mariam-ke
Copy link
Copy Markdown
Contributor Author

I just removed them.

Equals the dimensionality of the computed feature space.

random_state : int, RandomState instance or None, optional (default=None)
random_state : int, RandomState instance, 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.

since None has a very specific meaning in the case of random_state, we should still mention it as a possible value. It does not correspond to "nothing" in this case:

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changes done.

Equals the dimensionality of the computed feature space.

random_state : int, RandomState instance or None, optional (default=None)
random_state : int, RandomState instance, 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.

same thing here and in other places please @Mariam-ke ;)

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

Returns
-------
X_new : {array, sparse matrix}, \
X_new : {ndarray, 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 : {ndarray, sparse matrix}, \
X_new : {ndarray, sparse matrix}, \

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.

These are explicitly numpy arrays.

Suggested change
components_ : array-like of shape (n_components, n_features)
components_ : ndarray of shape (n_components, n_features)

Subset of training points used to construct the feature map.

component_indices_ : array, shape (n_components)
component_indices_ : array-like of shape (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.

Suggested change
component_indices_ : array-like of shape (n_components)
component_indices_ : ndarray of shape (n_components)

Indices of ``components_`` in the training set.

normalization_ : array, shape (n_components, n_components)
normalization_ : array-like of shape (n_components, 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.

Suggested change
normalization_ : array-like of shape (n_components, n_components)
normalization_ : ndarray of shape (n_components, n_components)

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.

Suggested change
X_transformed : array-like of shape (n_samples, n_components)
X_transformed : ndarray of shape (n_samples, n_components)

@Mariam-ke
Copy link
Copy Markdown
Contributor Author

Made all the changes

Equals the dimensionality of the computed feature space.

random_state : int, RandomState instance or None, optional (default=None)
rrandom_state : int, RandomState instance, 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
rrandom_state : int, RandomState instance, default=None
random_state : int, RandomState instance or None, default=None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

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 @Mariam-ke !

@reshamas
Copy link
Copy Markdown
Member

reshamas commented Jun 6, 2020

#DataUmbrella sprint

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you @Mariam-ke !

@thomasjpfan thomasjpfan merged commit aeffba3 into scikit-learn:master Jun 6, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
)

* updated file changes

* changed random state possible value

* changed random state possible value

* changed suggested changes

* changed suggested changes

* changed suggested changes

Co-authored-by: Mariam <mariam@Mariams-MacBook-Pro.local>
Co-authored-by: Mariam-ke <mariam.haji01@gmail.com>
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
)

* updated file changes

* changed random state possible value

* changed random state possible value

* changed suggested changes

* changed suggested changes

* changed suggested changes

Co-authored-by: Mariam <mariam@Mariams-MacBook-Pro.local>
Co-authored-by: Mariam-ke <mariam.haji01@gmail.com>
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.

5 participants