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

Possibly functionalize the plotting functions? #2141

Open
4 tasks
daquinteroflex opened this issue Jan 2, 2025 · 3 comments
Open
4 tasks

Possibly functionalize the plotting functions? #2141

daquinteroflex opened this issue Jan 2, 2025 · 3 comments
Labels

Comments

@daquinteroflex
Copy link
Collaborator

daquinteroflex commented Jan 2, 2025

Break this up:

  • Restructuring the plotting functions into viz
  • On the fly import of matplotlib
  • Eventually restructure the same functions to be composed using the (fig, axs) mechanism.
  • Possibly introduce standard object.plot functions using the viz module as an internal function call onto self.

Based on a conversation I had with Marc and my own personal experience:

Maybe it'd be nicer in terms of extensibility if we had purely functional plotting? Something on the lines of:

fig, axs = my_plotting_function(fig, axs, ...)
fig, axs = my_second_plotting_function(fig, axs, ...)

This can make it easier to represent information as just a set of operations between fig, axs and kwargs. We can move all the plotting functionality outside the classmethods and maybe make it more reusable? This way we avoid

def current_plotting_function():
    fig, ax = self.another_function(...)
    ax.do_something()
    self.another_function_in_this_class(fig, ax) # can mutate the fig, ax object directly and sometimes not accessible at a higher level
    return fig, axs
@momchil-flex
Copy link
Collaborator

I'm in favor of this, only curious about the exact scope especially from user perspective. If it's about moving all plotting functionality in a viz module while still keeping the same user-facing methods, then I think it sounds great. It's also in line with what we discussed about "properly" eliminating matplotlib as a core dependence.

@daquinteroflex
Copy link
Collaborator Author

Yeah that'd be great actually, and change the functional definitions accordingly when we do this change.

my_pseudo_static_classes/
viz/
   all_my_internal_functions/

and then refactor those functions in that module to be used in:

class MyClass:

   my_plotting_function -> (fig, ax) = viz.my_function() # just like before but also 

In principle, for backwards compatibility we can make my_plotting_function -> (fig, ax) return the same data as before with the new structure.

@momchil-flex
Copy link
Collaborator

momchil-flex commented Jan 3, 2025

Yeah, what I like about this apart from backwards compatibility is that otherwise it seems to me that we have two usage options, either:

viz.plot(object: Union[Simulation, Source, Monitor, ...])
# ugly-ish internal dispatch based on ``isinstance(object)``

or

viz.plot_simulation(simulation)
viz.plot_source(source)
viz.plot_monitor(monitor)
# a bit annoying to use

and both seem inferior to just object.plot().

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

No branches or pull requests

2 participants