r4fun / hierplane

🌳 Hierplane for R
https://r4fun.github.io/hierplane/
Other
9 stars 0 forks source link

Decouple spacy from build modules #16

Closed mathidachuk closed 4 years ago

mathidachuk commented 4 years ago

Decouple spacy from build and hierplane functions. Added wrapper function for spacy hierplane.

Resolves #6

tylerlittlefield commented 4 years ago

I am trying to get this to work and I think this is it right?

hierplane(
  x = spacy_df("sam likes boats"),
  title = "sam likes boats",
  settings = spacy_default()
)

I am not sure how I feel about this syntax because:

  1. The user needs to duplicate the text
  2. The user needs to provide spacyr settings

I think both of these requirements can be avoided. I think we need to make a stance that hierplane will always expect at least a dataframe but in most cases a dataframe and settings. The only time we would just need a dataframe is for the spacyr example where we conveniently handle the settings part for them. We could do this by looking at the class of the dataframe:

x <- spacyr::spacy_parse("sam likes boats")
class(x)

if (inherits(x, "spacyr_parsed")) {
  print("lets do spacyr stuff")
} else {
  print("lets not do spacyr stuff")
}

However, I can see the appeal of the old way, just providing a simple string of text. In this case, maybe we have hierplane_txt() for this and hierplane() for dataframes that aren't necessarily about visualizing text. We could sort of explain this to users by saying:

We offer two hierplane functions, one is hierplane() and it's used for visualizing any number of tree like dataframes (organization tree for example). The other is hierplane_txt() which is used for visualizing tokens in text, this function only requires a string of text.

Also, could you provide a reproducible example to demonstrate the decoupling?

mathidachuk commented 4 years ago

For Spacy usage, users should invoke hierplace_spacy, for which text is the only param required.

here's a full example on how this could work! (please pull before running) p.s. full disclaimer - did not even try to clean the documentation and stuff so rcmd check is failing :D p.p.s. construct_settings should probably go in utils.

EDIT NOTES:

a <- as.data.frame(dplyr::tribble( ~parent_id, ~child_id, ~child, ~node_type, ~link, ~height, ~age, "Bob", "Bob", "Bob", "gen1", "ROOT", "100 cm", "60 yo", "Bob", "Angelica", "Angelica", "gen2", "daughter", "100 cm", "30 yo", "Bob", "Eliza", "Eliza", "gen2", "daughter", "90 cm", "25 yo", "Bob", "Peggy", "Peggy", "gen2", "daughter", "50 cm", "10 yo", "Angelica", "John", "John", "gen3", "son", "10 cm", "0.5 yo" ))

hierplane(a, title = "Family Tree", settings = construct_settings(attributes = c("height", "age"))) hierplane_spacy("Bob has three daughters and a grandson.", theme = "dark")

tylerlittlefield commented 4 years ago

This looks good, I'm going to merge and focus on cleaning up RCMD check/removing spacyr dep