moodymudskipper / flow

View and Browse Code Using Flow Diagrams
https://moodymudskipper.github.io/flow/
Other
395 stars 26 forks source link

flow_view_deps() and flow_view_vars() should support scripts and expressions #88

Closed moodymudskipper closed 1 year ago

moodymudskipper commented 2 years ago

flow_view_vars() supports them through https://github.com/moodymudskipper/flow/commit/9a7c183d83230184e830302ad392bfd98ce635ba

Not super clean though!

moodymudskipper commented 2 years ago

After #101

moodymudskipper commented 1 year ago

Meh, let's wait for use cases or requests

llrs commented 1 year ago

I am interested in this feature. I am using {targets} with several files under R/*.R with some functions and I wanted to see their dependency graph (To see how many targets are affected if I modify a function). However, flow::flow_view_deps requires having a DESCRIPTION file which halted the functions:

traceback()
## 5: read.dcf(system.file("DESCRIPTION", package = pkg))
## 4: deps(fallback_ns_nm)
## 3: unique(c(namespaces, deps(fallback_ns_nm)))
## 2: flow_view_deps_df(target_fun_name, fun, trim, promote, demote, 
##        hide, lines, default_env = call_env)
## 1: flow::flow_view_deps(enrichment)

I can confirm that flow_view_vars works. I don't know if by not super clean you mean that some arrows cross other, or that it doesn't recognize something like return(c(a, b)) as a single output. But I like that if a variable is modified a * appears near it I'm using flow 0.2.0 from CRAN

moodymudskipper commented 1 year ago

Thanks! I will take a look then. There are some design decisions to make though:

(1) I could either search the files for function definitions, or assume they contain only functions. If they contain only functions it's easier, since I can just source those into an environment. disentangling messy scripts can be useful too though.

(2) dependencies. I could fetch all library() and require() calls present in the script and all pkg::fun instances. Assuming an order in the scripts sounds like a nightmare though so I think I'd need to assume packages are attached from the start.

flow_view_vars() is a bit of a mess. The last time I looked at it I didn't actually understand the code, so it's a bit embarrassing, but it seems to work OK, though I think the arrows don't always make perfect sense. Do you use this function regularly ?

About the layout of the diagram I can't do much, this is on nomnoml (I could change the engine), the return(c(a, b)) thing I'll have to take a look!

llrs commented 1 year ago
  1. I wouldn't just assume they only have functions. Some people like to write some constants in scripts that are just read once and then later on used as input for those functions.
  2. I'm not sure how the order affects the internals of flow. But anyway it is good practice to move them at the top, so I'd do that (but keep the order of appearance in mind).

I don't use flow_view_vars() regularly, but I might from now on. It might help to write better code or at least to learn about new code faster (when doing PR or working with other people code).

moodymudskipper commented 1 year ago

Let's close it and follow up in #138, seems to have a better scope