Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Where should the graph utilities lie? #575

Open
michaeldeistler opened this issue Jan 30, 2025 · 0 comments
Open

Where should the graph utilities lie? #575

michaeldeistler opened this issue Jan 30, 2025 · 0 comments

Comments

@michaeldeistler
Copy link
Contributor

Copied from #355

Jonas:
I decided to put everything in io.graph and the API to be from_graph(cell) / to_graph(cell).
However, we could also think about it moving this to the Modules i.e.

cell = jx.Cell()
cell.from_graph()
cell.to_graph()

This would probably also get rid of the circular import issues and the from_graph would not have to infer what type of module the graph has to be ported to. Also I like that you could call cell.to_graph() internally as well, to be used in plotting etc. should we want to keep around vis

I am open to discussing. Could also move the current swc_reader to cell.from_swc or sth.

Michael:
Let's keep it in io. I like that it is so neatly separated and more functional programming.
EDIT: Hm I do see the point of calling cell.to_graph() internally. Especially if we (as I suggest above) disallow any import io anywhere else. Not sure anymore now :D
I don't think I am a fan of cell.from_swc. This would then be sth like this:

cell = jx.Cell()
cell.from_swc() 

Which I find strange because, to me, modules should as much as possible be static in their graph defintion (otherwise nothing is tested at all and I would expect weird things to happen).

That's also why I really like the current API: one has to go via the graph to modify the graph definition of cell. I find that very intuitive.

Jonas:
Can keep this in the back of our minds. Might make sense to open an issue about the stuff above to keep track of this for future reference, once this is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant