scverse / spatialdata-plot

Static plotting for spatialdata
BSD 3-Clause "New" or "Revised" License
33 stars 13 forks source link

Feature request: remove need for intermediate pl calls when chaining renderers #54

Open kevinyamauchi opened 1 year ago

kevinyamauchi commented 1 year ago

Description of feature

Thanks for the super nice work, @timtreis and @sagar87 ! I think it would be nice if we didn't have to add intermediate calls to pl when chaining renderers. That is, I think it would be nice if instead of this:

sdata.pl.render_images().pl.render_shapes().pl.show()

We could do this:

sdata.pl.render_images().render_shapes().show()

This perhaps could be possible if the render_*() methods returned the plot accessor instead of the SpatialData object. I suppose this means that one couldn't do a pp operation after a pl operation, but maybe that suggests the pl and pp operations should be on the same level of the hierarchy or one should do their preprocessing first, then their plotting. I'm not sure what's best here, but it might be good to experiment with this once all of the p0 functionality is finished.

timtreis commented 1 year ago

Would this be coherent with the syntax from other parts of the package? I'm not super aware of the other modules but is the syntax right now

sdata
  .pp.get_bb()
  .pp.query()

or

sdata
  .pp
      .get_bb()
      .query()

?

I'm sure we could find a way in which we'd only need to call the plot accessor once, but I'd prefer not to implement sth that could raise the question "when and how many pl, pp etc do I need?"

LucaMarconato commented 1 year ago

Related also to this https://github.com/scverse/spatialdata/issues/225, we need to discuss some more uniform names/syntax. We could do this next week at the hackathon.