ani-hovhannisyan / kanji-visualization

Kanji words visualization graph draws relational graph for kanjis of particular words in Japanese. Aim is to understand the relational graph of one kanji within different words and it's relations to all possible words.
MIT License
5 stars 1 forks source link

Implemented draft nodes graph creation in GraphController #59

Closed ani-hovhannisyan closed 2 years ago

ani-hovhannisyan commented 2 years ago

Added #43 #36 related json format creation in GraphController.py. Current code has many bad coding. I see the dev locally but it's not visible from here, can we merge to main? Asigning everyone as reviewer, as more eyes see more problems ))). To test working flow, just do get request via kanji get API passing like (山、川) as these are the only kanjis placed in sample json which now is considered as DB (the biggest one contains unecesary data, I plan to clean up it). If you pass proper json file instead, the GC will reconstruct it as necessary, see pics from #36 for GraphView. The final response consists some key values which will remove later, as these were left from other classes than GC. There are many problems, so please add as much as possible comments so I can concentrate on them too.

wowry commented 2 years ago

@ani-hovhannisyan So far, your GraphContoller seems to be able to work well with my GraphView, so I think it is OK.

image

And sorry, I didn't take into account the different ways of writing boolean in js and python (true vs True). In order to be consistent with GraphContoller, I would like to change the type of isMain from boolean to string and use the expression isMain === "true" to distinguish the main node. No action is required from you due to this change.

ani-hovhannisyan commented 2 years ago

@ani-hovhannisyan So far, your GraphContoller seems to be able to work well with my GraphView, so I think it is OK.

image

And sorry, I didn't take into account the different ways of writing boolean in js and python (true vs True). In order to be consistent with GraphContoller, I would like to change the type of isMain from boolean to string and use the expression isMain === "true" to distinguish the main node. No action is required from you due to this change.

Yes it works with view fine. I'm cleaning up unnecessary key values now. For tru yes that was also the concern. But I guessed it's necessary for graph library so made string not boolen. Thanks for your effords.

wowry commented 2 years ago

I'm cleaning up unnecessary key values now.

Yes, please. Thank you very much!!

ani-hovhannisyan commented 2 years ago

Btw, do you have any idea of drawing last furigana nodes? I didnt know how to represent them, in a one node? or left as is now?

wowry commented 2 years ago

@ani-hovhannisyan Sorry for the late reply. Is it possible to write this way?

{
  "nodes": [
    { "id": "きゅう" },
    { "id": "川", "isMain": "true" }
  ],
  "links": [
    { "source": "川", "target": "きゅう" }
  ]
}

GraphView supports this way of writing, but I need to adjust the style of the nodes (e.g. width).

ani-hovhannisyan commented 2 years ago

@ani-hovhannisyan Sorry for the late reply. Is it possible to write this way?

{
  "nodes": [
    { "id": "きゅう" },
    { "id": "川", "isMain": "true" }
  ],
  "links": [
    { "source": "川", "target": "きゅう" }
  ]
}

GraphView supports this way of writing, but I need to adjust the style of the nodes (e.g. width).

Yes, will change that part, thanks. That's better than current representation.

ani-hovhannisyan commented 2 years ago

Converted all kanji data, now backend will use all kanji data json.

ani-hovhannisyan commented 2 years ago

The docker fails due to no jamdict existance in requirements, shall I add it as it's used in InfoController or to wait until info-controller branch will be merged?

wowry commented 2 years ago

Yes, such action will be required, but actually, InfoController has been git reset --hard by @teruto725 because @TakumiHiraide unintentionally pushed the wrong code to main.

You will see the following message above.

teruto725 force-pushed the main branch from 1e28f70 to 2ffd192 15 hours ago

(I think this change should have been git revert instead of git reset so that other members can trace the change.)

So the InfoContoller code you have now needs to be discarded to prevent conflicts. You might need to copy the previous InfoController code from GitHub and commit it.

wowry commented 2 years ago

Btw, do you have any idea of drawing last furigana nodes? I didnt know how to represent them, in a one node? or left as is now?

I came to think that handling hiragana might be contrary to the design. It is possible to handle hiragana itself, but I think only kanji was considered in the design phase.

For example, in our domain model:

The graphical view field has
-        main node (searched kanji)
-        friend node (pair kanji in the same word)    <--- THIS ONE
-        edge (pointing to friend node)

In order to follow this domain model, we will be limited to dealing with words that consist only of kanji. Or does it need to be redesigned?

ani-hovhannisyan commented 2 years ago

Btw, do you have any idea of drawing last furigana nodes? I didnt know how to represent them, in a one node? or left as is now?

I came to think that handling hiragana might be contrary to the design. It is possible to handle hiragana itself, but I think only kanji was considered in the design phase.

For example, in our domain model:

The graphical view field has
-        main node (searched kanji)
-        friend node (pair kanji in the same word)    <--- THIS ONE
-        edge (pointing to friend node)

In order to follow this domain model, we will be limited to dealing with words that consist only of kanji. Or does it need to be redesigned?

Right. Agree. I forgot that design. Then I'll skip all the furigana words. But will add some Todo. If necessary handle it.

ani-hovhannisyan commented 2 years ago

About this

So the InfoContoller code you have now needs to be discarded to prevent conflicts. You might need to copy the previous InfoController code from GitHub and commit it.

I guess all already merged into GC from IC branch, and fixed PR fails, need only to be reviewed.