You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tillrohrmann <gi...@git.apache.org> on 2015/11/17 18:28:52 UTC

[GitHub] flink pull request: [FLINK-3036] [gelly] Fix Graph.fromCsvReader m...

GitHub user tillrohrmann opened a pull request:

    https://github.com/apache/flink/pull/1370

    [FLINK-3036] [gelly] Fix Graph.fromCsvReader method in Gelly's Scala API

    The Graph.fromCsvReader in Gelly's Scala API returns a wrongly typed Graph instance because
    the implementation contains code paths with return types which are not compatible. This
    lead to a bad user experience.
    
    This commit fixes this. However, it also introduces a slightly different behaviour and is
    thus API breaking. Before there were parameters hasEdgeValues and readVertices which
    controlled whether edge values and vertices shall be read. This is now implicitly controlled
    by the types of the vertex and edge value. If the type is NullValue for the edge values then
    the edge value won't be read from the given csv file. If the vertex value type is NullValue,
    then all vertices will have a value of a NullValue instance. If the pathVertices is not
    specified and the vertex value type is unequal to NullValue, then the vertexValueInitializer
    is used for the initialization.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tillrohrmann/flink fixGelly

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1370.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 #1370
    
----
commit 2edec702ceac62ff8bbcdf7b6dda8f029d5f2bea
Author: Till Rohrmann <tr...@apache.org>
Date:   2015-11-17T17:21:47Z

    [FLINK-3036] [gelly] Fix Graph.fromCsvReader method in Gelly's Scala API
    
    The Graph.fromCsvReader in Gelly's Scala API returns a wrongly typed Graph instance because
    the implementation contains code paths with return types which are not compatible. This
    lead to a bad user experience.
    
    This commit fixes this. However, it also introduces a slightly different behaviour and is
    thus API breaking. Before there were parameters hasEdgeValues and readVertices which
    controlled whether edge values and vertices shall be read. This is now implicitly controlled
    by the types of the vertex and edge value. If the type is NullValue for the edge values then
    the edge value won't be read from the given csv file. If the vertex value type is NullValue,
    then all vertices will have a value of a NullValue instance. If the pathVertices is not
    specified and the vertex value type is unequal to NullValue, then the vertexValueInitializer
    is used for the initialization.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3036] [gelly] Fix Graph.fromCsvReader m...

Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:

    https://github.com/apache/flink/pull/1370#issuecomment-157500925
  
    Thanks a lot for fixing this @tillrohrmann!
    I actually like the new behavior better ;) Could you please also update the gelly guide?
    One more thing, some of the formatting changes you've made are also present in #1356. I was planning to merge it, but you'll have to rebase this one. Let me know if that's OK or whether it's more convenient for you to merge #1356 and this one yourself. Thanks 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3036] [gelly] Fix Graph.fromCsvReader m...

Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:

    https://github.com/apache/flink/pull/1370#issuecomment-157659857
  
    Awesome, thanks Till!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3036] [gelly] Fix Graph.fromCsvReader m...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1370


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-3036] [gelly] Fix Graph.fromCsvReader m...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:

    https://github.com/apache/flink/pull/1370#issuecomment-157657885
  
    @vasia, I can take care of merging both PRs. The overlapping parts don't seem to be too difficult to resolve.
    
    Will update the gelly guide before merging.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---