[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/1829 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-210647072 Thanks for the update @smarthi. Will merge this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-210080951 I think it will go to 1.1.x release. Need some clean up to isolate the changes needed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user andrewpalumbo commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-210057521 Is it possible to get this merged into the 1,0.2 release? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
GitHub user smarthi reopened a pull request: https://github.com/apache/flink/pull/1829 FLINK-3657: Change access of DataSetUtils.countElements() to 'public' You can merge this pull request into a Git repository by running: $ git pull https://github.com/smarthi/flink Flink-3657 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/1829.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1829 commit a3e33aa7bc8fad09a122490a13d294210df5e186 Author: smarthiDate: 2016-03-23T06:28:28Z FLINK-3657: Change access of DataSetUtils.countElements() to 'public' --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-205001401 +1 for merging to master. But would love to isolate the changes to exclude refactoring changes if possible --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user uce commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-204931011 @smarthi I think it's not too specific at all since its part of `DataSetUtils` and not `DataSet`. Let's merge the changes to master for `1.1`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user smarthi commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-204762444 Closing this PR without merging as its "way too specific", thanks for all the feedback. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user smarthi closed the pull request at: https://github.com/apache/flink/pull/1829 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-204614791 @mbalassi : There is already request from @zentol and me to clean up the PR, so would love to get this merge excluding the refactor parts. Thanks. I think we should merge this to master and 1.1.0, the question is whether this should go to 1.0.1 release, which @fhueske and @uce think should not. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user mbalassi commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-204587443 I think @smarthi has a point that the functionality is a nice fit for `DataSetUtils` and he has properly added it to the scala API and covered the tests. It is true that it has some minor refactoring obfuscating the core PR, but besides that LGTM. If noone thinks otherwise I would merge this Tuesday morning with optionally excluding the refactor parts if it is considered that bothersome. Unfortunately for the Mahout release I think the policy is as @fhueske suggests that we should reserve new features for the next, 1.1.0 release line. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-201568146 @smarthi Could you remove some of the refactoring or clean up changes included with this PR? It can be added as separate PR please. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-201567738 If it is on the Dataset or any extendible public APIs, I would be worried. The change was to expose private to public without modifying existing behavior. Would be ok from my side. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user andrewpalumbo commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-201507914 I've actually come across several use cases for this when Implementing a blockified matrix multiplication operator. Exposing this method as public would be very helpful. It would be great to have in your 1.0.1 release. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user smarthi commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200848523 @uce @fhueske to add more context to this PR, we r in the final stretch of a planned 0.12.0 Mahout release that adds Flink as a distributed engine for Samsara linear algebra. I will be demoing the Mahout-Flink integration at my talk in ApacheCon, Vancouver, May 9-11. If this can't make it in 1.0.1, I guess we need to go with a redundant clone in Mahout until this becomes available in a future Flink release and want to avoid that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200804264 @uce, API stability is not an issue. The method was `private` before and part of the accessible API. IMO, the question is rather whether we want to add new features in a bugfix release. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user uce commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200803091 As a side note (I didn't look at the code changes): We can't rename the method name for `1.0.1` as `DataSetUtils` is annotated with `PublicEvolving`, meaning that we are only allowed to change between minor versions, that is for `1.1.0`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user smarthi commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200551422 Not sure as to why u think this is "way to specific", this is a very convenient feature to have when dealing with Distributed Row Matrix (DRM) blocks for distributed Machine Learning computations. We found ourselves 3 different variants doing the same thing and Flink happened to have one that directly did it for Flink's datasets. Hence this PR. @fhueske thanks for your feedback, it would be great to have this in the planned 1.0.1 bug release. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200418755 Please stop force pushing for squashed changes, it obfuscates which comments you addressed, forcing us to through the _entire _PR again. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user andrewpalumbo commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200413224 This method would be very useful to us, e.g., when assigning ordinal indices to the elements of a `DataSet` from an external mapping. I would not think that this is too specific a use case. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200244139 I'm not sure if it is too specific. The fact that somebody reimplemented the functionality hints that it might be useful. However, the name would need to be changed to something like `countElementsPerPartition()`, IMO. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
Github user zentol commented on the pull request: https://github.com/apache/flink/pull/1829#issuecomment-200215310 I think this method's use-case is **way** too specific to offer it as part of the API. You also touch 3 files purely for formatting. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...
GitHub user smarthi opened a pull request: https://github.com/apache/flink/pull/1829 FLINK-3657: Change access of DataSetUtils.countElements() to 'public' You can merge this pull request into a Git repository by running: $ git pull https://github.com/smarthi/flink Flink-3657 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/1829.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1829 commit d5ba6d90d3cdd6b93f962f2d9226e443810b08b0 Author: smarthiDate: 2016-03-23T06:28:28Z FLINK-3657: Change access of DataSetUtils.countElements() to 'public' --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---