search

apache / incubator-graphar

An open source, standard data file format for graph data storage and retrieval.
https://graphar.apache.org/
Apache License 2.0
192 stars 40 forks source link

feat(c++,spark): support json payload file format #518

Closed amygbAI closed 6 days ago

amygbAI commented 2 weeks ago

Reason for this PR

This is a rebased pull request, so to speak of ( due to the issue with JSONOptions being private in spark 3.2 and 3.3 ) ..also added test artifacts separately in incubator-graphar-testing. Was able to compile with spark 3.2 and 3.3 ( in both datasources-32 and datasources-33 )

What changes are included in this PR?

Changes in datasources-32 and datasources-33 Changes in cpp Changes in pyspark

Are these changes tested?

yes, added the generated ldbc samples in incubator-graphar-testing/ldbc_sample/json

Are there any user-facing changes?

not that i am aware of

None

SemyonSinchenko commented 2 weeks ago

@amygbAI I see that all the tests are passed except Neo4j example and the error is about missing test data:

Exception in thread "main" org.apache.spark.sql.AnalysisException: Path does not exist: file:/datadrive/APACHE_GRAPHAR/incubator-graphar/testing/ldbc_sample/person_0_0.csv

I mean, that code is compiled and there is no problem you mentioned in #488

amygbAI commented 2 weeks ago

thanks at @SemyonSinchenko .. pulled and removed the local reference .. is there some way to run all of these builds before opening a PR ? as per directions on the thread and README's i built all the directories where i had made changes and tested ..

acezen commented 2 weeks ago

thanks at @SemyonSinchenko .. pulled and removed the local reference .. is there some way to run all of these builds before opening a PR ? as per directions on the thread and README's i built all the directories where i had made changes and tested ..

Hi, @amygbAI, The C++ library you can reference to the https://github.com/apache/incubator-graphar/tree/main/cpp#building to check the test, and for format error, remember that we use the clang-format-8 for format check.

amygbAI commented 2 weeks ago

resolved all comments, formatted, linted and built using instructions .. also removed the "build" folder from cpp since it wasn't in the repository ( much like target folders )

SemyonSinchenko commented 2 weeks ago

May we add also a test for graphar spark+json?

amygbAI commented 2 weeks ago

please send me an example for spark + csv and i ll refactor it

On Wed, Jun 12, 2024 at 2:51 PM Semyon @.***> wrote:

May we add also a test for graphar spark+json?

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2162529195, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 2 weeks ago

please send me an example for spark + csv and i ll refactor it … On Wed, Jun 12, 2024 at 2:51 PM Semyon @.> wrote: May we add also a test for graphar spark+json? — Reply to this email directly, view it on GitHub <#518 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU . You are receiving this because you were mentioned.Message ID: @.>

You can refer to the existed test case an just add a test with updated json test data like:

json test can be:

test("read vertex chunks") {
// construct the vertex information
    val prefix = testData + "/ldbc_sample/json/"
    val vertex_yaml = prefix + "Person.vertex.yml"
    val vertex_info = VertexInfo.loadVertexInfo(vertex_yaml, spark)

    // construct the vertex reader
    val reader = new VertexReader(prefix, vertex_info, spark)

    // test reading the number of vertices
    assert(reader.readVerticesNumber() == 903)
    val property_group = vertex_info.getPropertyGroup("gender")

    // test reading a single property chunk
    val single_chunk_df = reader.readVertexPropertyChunk(property_group, 0)
    assert(single_chunk_df.columns.length == 4)
    assert(single_chunk_df.count() == 100)
    val cond = "gender = 'female'"
    var df_pd = single_chunk_df.select("firstName", "gender").filter(cond)

json test can be:

def test_vertex_reader_with_json(spark):
    initialize(spark)

    vertex_info = VertexInfo.load_vertex_info(
        GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/")
        .joinpath("Person.vertex.yml")
        .absolute()
        .__str__()
    )
    vertex_reader = VertexReader.from_python(
        GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/").absolute().__str__(),
        vertex_info,
    )
    assert VertexReader.from_scala(vertex_reader.to_scala()) is not None
    assert vertex_reader.read_vertices_number() > 0
    assert (
        vertex_reader.read_vertex_property_group(
            vertex_info.get_property_group("name")
        ).count()
        > 0
    )
    assert (
        vertex_reader.read_vertex_property_chunk(
            vertex_info.get_property_groups()[0], 0
        ).count()
        > 0
    )
    assert (
        vertex_reader.read_all_vertex_property_groups().count()
        >= vertex_reader.read_vertex_property_group(
            vertex_info.get_property_group("age")
        ).count()
    )
    assert (
        vertex_reader.read_multiple_vertex_property_groups(
            [vertex_info.get_property_group("name")]
        ).count()
        > 0
    )
amygbAI commented 2 weeks ago

alritey folks .. done

On Wed, Jun 12, 2024 at 4:39 PM Weibin Zeng @.***> wrote:

please send me an example for spark + csv and i ll refactor it … <#m-7895995846438427138> On Wed, Jun 12, 2024 at 2:51 PM Semyon @.> wrote: May we add also a test for graphar spark+json? — Reply to this email directly, view it on GitHub <#518 (comment) https://github.com/apache/incubator-graphar/pull/518#issuecomment-2162529195>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU . You are receiving this because you were mentioned.Message ID: @.>

You can refer to the existed test case an just add a test with updated json test data like:

json test can be:

test("read vertex chunks") {// construct the vertex information val prefix = testData + "/ldbc_sample/json/" val vertex_yaml = prefix + "Person.vertex.yml" val vertex_info = VertexInfo.loadVertexInfo(vertex_yaml, spark)

// construct the vertex reader
val reader = new VertexReader(prefix, vertex_info, spark)

// test reading the number of vertices
assert(reader.readVerticesNumber() == 903)
val property_group = vertex_info.getPropertyGroup("gender")

// test reading a single property chunk
val single_chunk_df = reader.readVertexPropertyChunk(property_group, 0)
assert(single_chunk_df.columns.length == 4)
assert(single_chunk_df.count() == 100)
val cond = "gender = 'female'"
var df_pd = single_chunk_df.select("firstName", "gender").filter(cond)

json test can be:

def test_vertex_reader_with_json(spark): initialize(spark)

vertex_info = VertexInfo.load_vertex_info(
    GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/")
    .joinpath("Person.vertex.yml")
    .absolute()
    .__str__()
)
vertex_reader = VertexReader.from_python(
    GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/").absolute().__str__(),
    vertex_info,
)
assert VertexReader.from_scala(vertex_reader.to_scala()) is not None
assert vertex_reader.read_vertices_number() > 0
assert (
    vertex_reader.read_vertex_property_group(
        vertex_info.get_property_group("name")
    ).count()
    > 0
)
assert (
    vertex_reader.read_vertex_property_chunk(
        vertex_info.get_property_groups()[0], 0
    ).count()
    > 0
)
assert (
    vertex_reader.read_all_vertex_property_groups().count()
    >= vertex_reader.read_vertex_property_group(
        vertex_info.get_property_group("age")
    ).count()
)
assert (
    vertex_reader.read_multiple_vertex_property_groups(
        [vertex_info.get_property_group("name")]
    ).count()
    > 0
)

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2162737305, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSCZKVPDSRXA7AYB56DZHAT7JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG4ZTOMZQGU . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 2 weeks ago

alritey folks .. done On Wed, Jun 12, 2024 at 4:39 PM Weibin Zeng @.> wrote: … please send me an example for spark + csv and i ll refactor it … <#m-7895995846438427138> On Wed, Jun 12, 2024 at 2:51 PM Semyon @.> wrote: May we add also a test for graphar spark+json? — Reply to this email directly, view it on GitHub <#518 (comment) <#518 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU . You are receiving this because you were mentioned.Message ID: @.> You can refer to the existed test case an just add a test with updated json test data like: - Spark https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/maven-projects/spark/graphar/src/test/scala/org/apache/graphar/TestReader.scala#L99-L117 json test can be: test("read vertex chunks") {// construct the vertex information val prefix = testData + "/ldbc_sample/json/" val vertex_yaml = prefix + "Person.vertex.yml" val vertex_info = VertexInfo.loadVertexInfo(vertex_yaml, spark) // construct the vertex reader val reader = new VertexReader(prefix, vertex_info, spark) // test reading the number of vertices assert(reader.readVerticesNumber() == 903) val property_group = vertex_info.getPropertyGroup("gender") // test reading a single property chunk val single_chunk_df = reader.readVertexPropertyChunk(property_group, 0) assert(single_chunk_df.columns.length == 4) assert(single_chunk_df.count() == 100) val cond = "gender = 'female'" var df_pd = single_chunk_df.select("firstName", "gender").filter(cond) - pyspark https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/pyspark/tests/test_reader.py#L29-L67 json test can be: def test_vertex_reader_with_json(spark): initialize(spark) vertex_info = VertexInfo.load_vertex_info( GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/") .joinpath("Person.vertex.yml") .absolute() .str() ) vertex_reader = VertexReader.from_python( GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/").absolute().str(), vertex_info, ) assert VertexReader.from_scala(vertex_reader.to_scala()) is not None assert vertex_reader.read_vertices_number() > 0 assert ( vertex_reader.read_vertex_property_group( vertex_info.get_property_group("name") ).count() > 0 ) assert ( vertex_reader.read_vertex_property_chunk( vertex_info.get_property_groups()[0], 0 ).count() > 0 ) assert ( vertex_reader.read_all_vertex_property_groups().count() >= vertex_reader.read_vertex_property_group( vertex_info.get_property_group("age") ).count() ) assert ( vertex_reader.read_multiple_vertex_property_groups( [vertex_info.get_property_group("name")] ).count() > 0 ) — Reply to this email directly, view it on GitHub <#518 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSCZKVPDSRXA7AYB56DZHAT7JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG4ZTOMZQGU . You are receiving this because you were mentioned.Message ID: @.>

Hi, @amygbAI Can you add me to your graphar fork repo's collaborator? I can help you fix the format and test.

amygbAI commented 2 weeks ago

done ..kindly let me know what changes u make .. i am compiling a document of all the communication we have had so that it becomes a good starting point for noobs like me

On Mon, Jun 17, 2024 at 8:43 AM Weibin Zeng @.***> wrote:

alritey folks .. done On Wed, Jun 12, 2024 at 4:39 PM Weibin Zeng @.

*> wrote: … <#m-8019765962861073975> please send me an example for spark

Hi, @amygbAI https://github.com/amygbAI Can you add me to your graphar folk repo's collaborator? I can help you fix the format and test.

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2172094296, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSC245NKCVPWVESRACTZHZH5JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZSGA4TIMRZGY . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 1 week ago

done ..kindly let me know what changes u make .. i am compiling a document of all the communication we have had so that it becomes a good starting point for noobs like me On Mon, Jun 17, 2024 at 8:43 AM Weibin Zeng @.> wrote: … alritey folks .. done On Wed, Jun 12, 2024 at 4:39 PM Weibin Zeng @. > wrote: … <#m-8019765962861073975> please send me an example for spark + csv and i ll refactor it … <#m-7895995846438427138> On Wed, Jun 12, 2024 at 2:51 PM Semyon @.> wrote: May we add also a test for graphar spark+json? — Reply to this email directly, view it on GitHub <#518 <#518> (comment) <#518 (comment) <#518 (comment)>>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU https://github.com/notifications/unsubscribe-auth/ATIQOSBBSXKDTSQNNEBHTVLZHAHJ3AVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSGUZDSMJZGU . You are receiving this because you were mentioned.Message ID: @.> You can refer to the existed test case an just add a test with updated json test data like: - Spark https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/maven-projects/spark/graphar/src/test/scala/org/apache/graphar/TestReader.scala#L99-L117 https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/maven-projects/spark/graphar/src/test/scala/org/apache/graphar/TestReader.scala#L99-L117 json test can be: test("read vertex chunks") {// construct the vertex information val prefix = testData + "/ldbc_sample/json/" val vertex_yaml = prefix + "Person.vertex.yml" val vertex_info = VertexInfo.loadVertexInfo(vertex_yaml, spark) // construct the vertex reader val reader = new VertexReader(prefix, vertex_info, spark) // test reading the number of vertices assert(reader.readVerticesNumber() == 903) val property_group = vertex_info.getPropertyGroup("gender") // test reading a single property chunk val single_chunk_df = reader.readVertexPropertyChunk(property_group, 0) assert(single_chunk_df.columns.length == 4) assert(single_chunk_df.count() == 100) val cond = "gender = 'female'" var df_pd = single_chunk_df.select("firstName", "gender").filter(cond) - pyspark https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/pyspark/tests/test_reader.py#L29-L67 https://github.com/apache/incubator-graphar/blob/ff057cf9bfb3325e2822b31a3e3041e3314c2d97/pyspark/tests/test_reader.py#L29-L67 json test can be: def test_vertex_reader_with_json(spark): initialize(spark) vertex_info = VertexInfo.load_vertex_info( GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/") .joinpath("Person.vertex.yml") .absolute() .str() ) vertex_reader = VertexReader.from_python( GRAPHAR_TESTS_EXAMPLES.joinpath("/ldbc_sample/json/").absolute().str(), vertex_info, ) assert VertexReader.from_scala(vertex_reader.to_scala()) is not None assert vertex_reader.read_vertices_number() > 0 assert ( vertex_reader.read_vertex_property_group( vertex_info.get_property_group("name") ).count() > 0 ) assert ( vertex_reader.read_vertex_property_chunk( vertex_info.get_property_groups()[0], 0 ).count() > 0 ) assert ( vertex_reader.read_all_vertex_property_groups().count() >= vertex_reader.read_vertex_property_group( vertex_info.get_property_group("age") ).count() ) assert ( vertex_reader.read_multiple_vertex_property_groups( [vertex_info.get_property_group("name")] ).count() > 0 ) — Reply to this email directly, view it on GitHub <#518 (comment) <#518 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSCZKVPDSRXA7AYB56DZHAT7JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG4ZTOMZQGU https://github.com/notifications/unsubscribe-auth/ATIQOSCZKVPDSRXA7AYB56DZHAT7JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRSG4ZTOMZQGU . You are receiving this because you were mentioned.Message ID: @.**> Hi, @amygbAI https://github.com/amygbAI Can you add me to your graphar folk repo's collaborator? I can help you fix the format and test. — Reply to this email directly, view it on GitHub <#518 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSC245NKCVPWVESRACTZHZH5JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZSGA4TIMRZGY . You are receiving this because you were mentioned.Message ID: @.>

the GraphAr C++ CI is failed in format check. Can you double check the clang-format is version 8.x.x with command

clang-format --version
acezen commented 1 week ago

BTW, You can watch the Details, there are the reports for the CI/CD, you can check the report and find the reason why the CI failed.

截屏2024-06-17 15 54 59
amygbAI commented 1 week ago

ok, pushed .. though i am not sure why this would fail for certain OS and not the others ..also can this test directory also please be included in the CMakeLists.txt

On Mon, Jun 17, 2024 at 1:30 PM Weibin Zeng @.***> wrote:

BTW, You can watch the Details, there are the reports for the CI/CD, you can check the report and find the reason why the CI failed. 2024-06-17.15.54.59.png (view on web) https://github.com/apache/incubator-graphar/assets/11835645/388c7151-6a2f-4795-a9e9-204dde95350a

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2172565507, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSAJ5WQQN4DZCQKRRJLZH2JTHAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZSGU3DKNJQG4 . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 1 week ago

Hi, @amygbAI , I have fix the c++ format with clang-format-8, and do some changes:

acezen commented 1 week ago

Hi， @amygbAI， I think the the PR is overall LGTM, you may fix the remaining problem base on Sem's comments.

amygbAI commented 1 week ago

after making the changes when i tried to push git showed me some conflicts and after gpt + google search i followed the below

git stash git pull origin 170-feat-support-json-payload-file-format git stash pop git add . git commit -m "msg" git push origin 170-feat-support-json-payload-file-format

for some reason now it shows some conflict in the scripts file AND also in the cpp file filesystem.cc .. totally confused

acezen commented 1 week ago

after making the changes when i tried to push git showed me some conflicts and after gpt + google search i followed the below

git stash git pull origin 170-feat-support-json-payload-file-format git stash pop git add . git commit -m "msg" git push origin 170-feat-support-json-payload-file-format

for some reason now it shows some conflict in the scripts file AND also in the cpp file filesystem.cc .. totally confused

That's because your branch and main modified the same file. You need to rebase the main first and fix the conflict: refer: https://www.atlassian.com/git/tutorials/merging-vs-rebasing

acezen commented 1 week ago

after making the changes when i tried to push git showed me some conflicts and after gpt + google search i followed the below

git stash git pull origin 170-feat-support-json-payload-file-format git stash pop git add . git commit -m "msg" git push origin 170-feat-support-json-payload-file-format

for some reason now it shows some conflict in the scripts file AND also in the cpp file filesystem.cc .. totally confused

Or just revert the maven-projects/spark/scripts/run-ldbc-sample2graphar.sh to csv, we don't need to change this file.

amygbAI commented 1 week ago

thanks .. done

On Fri, Jun 21, 2024 at 1:08 PM Weibin Zeng @.***> wrote:

after making the changes when i tried to push git showed me some conflicts and after gpt + google search i followed the below

git stash git pull origin 170-feat-support-json-payload-file-format git stash pop git add . git commit -m "msg" git push origin 170-feat-support-json-payload-file-format

for some reason now it shows some conflict in the scripts file AND also in the cpp file filesystem.cc .. totally confused

Or just revert the maven-projects/spark/scripts/run-ldbc-sample2graphar.sh to csv, we don't need to change this file.

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2182183114, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSHO7OVVWRTKIFXGY4TZIPJ6JAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBSGE4DGMJRGQ . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 1 week ago

origin

Hi, @amygbAI, seems that you have reverted the changes that I made ╥﹏╥

amygbAI commented 1 week ago

dang .. can u just give me the exact version on which u made the changes, i ll do a hard reset to that version and then add my final changes on it ..

On Mon, Jun 24, 2024 at 8:19 AM Weibin Zeng @.***> wrote:

origin

Hi, @amygbAI https://github.com/amygbAI, seems that you have reverted the changes that I made ╥﹏╥

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2185487349, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBZPJQX7X6WZFE5OQTZI6CMDAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGQ4DOMZUHE . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 1 week ago

dang .. can u just give me the exact version on which u made the changes, i ll do a hard reset to that version and then add my final changes on it .. … On Mon, Jun 24, 2024 at 8:19 AM Weibin Zeng @.> wrote: origin Hi, @amygbAI https://github.com/amygbAI, seems that you have reverted the changes that I made ╥﹏╥ — Reply to this email directly, view it on GitHub <#518 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSBZPJQX7X6WZFE5OQTZI6CMDAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGQ4DOMZUHE . You are receiving this because you were mentioned.Message ID: @.>

It's OK now. I have reset to the commit and apply your final change to it. And it should be OK now.

amygbAI commented 1 week ago

appreciate all your patience .. i think i made it harder than it actually should have been

On Mon, Jun 24, 2024 at 8:42 AM Weibin Zeng @.***> wrote:

@.**** approved this pull request.

LGTM, thanks for the work! Hi, @SemyonSinchenko https://github.com/SemyonSinchenko, this change is OK to me, could you give a review again?

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#pullrequestreview-2134543954, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSAAHSH74FYMCWJG7H3ZI6FCVAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMZUGU2DGOJVGQ . You are receiving this because you were mentioned.Message ID: @.***>

acezen commented 1 week ago

appreciate all your patience .. i think i made it harder than it actually should have been … On Mon, Jun 24, 2024 at 8:42 AM Weibin Zeng @.> wrote: @*.*** approved this pull request. LGTM, thanks for the work! Hi, @SemyonSinchenko https://github.com/SemyonSinchenko, this change is OK to me, could you give a review again? — Reply to this email directly, view it on GitHub <#518 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSAAHSH74FYMCWJG7H3ZI6FCVAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMZUGU2DGOJVGQ . You are receiving this because you were mentioned.Message ID: @.>

Don't be bothered about that. Since this is your first time to contribute to a open source project, your continue contribution and the final work are excellent. Hope you can join the community and continue to contribute to GraphAr.:)

amygbAI commented 1 week ago

appreciate it.. will be on the lookout to help with further dev and bug fixes !

On Mon, Jun 24, 2024 at 10:46 AM Weibin Zeng @.***> wrote:

appreciate all your patience .. i think i made it harder than it actually should have been … <#m-4654679496454462024> On Mon, Jun 24, 2024 at 8:42 AM Weibin Zeng @.*> wrote: @.* approved this pull request. LGTM, thanks for the work! Hi, @SemyonSinchenko https://github.com/SemyonSinchenko https://github.com/SemyonSinchenko, this change is OK to me, could you give a review again? — Reply to this email directly, view it on GitHub <#518 (review) https://github.com/apache/incubator-graphar/pull/518#pullrequestreview-2134543954>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSAAHSH74FYMCWJG7H3ZI6FCVAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMZUGU2DGOJVGQ . You are receiving this because you were mentioned.Message ID: @*.***>

Don't be bothered about that. Since this is your first time to contribute to a open source project, your continue contribution and the final work are excellent. Hope you can join the community and continue to contribute to GraphAr.:)

— Reply to this email directly, view it on GitHub https://github.com/apache/incubator-graphar/pull/518#issuecomment-2185623887, or unsubscribe https://github.com/notifications/unsubscribe-auth/ATIQOSHGEOEWXGDWCZOPKXDZI6TSDAVCNFSM6AAAAABJDPBQKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGYZDGOBYG4 . You are receiving this because you were mentioned.Message ID: @.***>

SemyonSinchenko commented 6 days ago

I will review it again in the evening.