glotzerlab / freud

Powerful, efficient particle trajectory analysis in scientific Python.
https://freud.readthedocs.io
BSD 3-Clause "New" or "Revised" License
270 stars 49 forks source link

More options to color Voronoi diagram by #991

Closed dnusha1994 closed 1 year ago

dnusha1994 commented 2 years ago

Is your feature request related to a problem? Please describe. Using Freud to do Voronoi tesselation of lipid bilayer phosphates

Describe the solution you'd like It would be useful to have the option to colour the Voronoi polygons by their area as well as by the number of sides they have. Is there any way this could be achieved?

Describe alternatives you've considered

Additional context

tommy-waltmann commented 2 years ago

Yes, that would be a simple and useful addition.

The ideal solution to achieve this would involve changing the argument color_by_sides for the Voronoi.plot method. It would need to be generalized to something like color_by, which instead of taking a bool, would take one of a set of acceptable strings (i.e. color_by='sides', or color_by='volume', etc.).

This proposed solution would be API breaking, though, so it would have to be pushed to a v3 release. A non-API breaking way to handle this would be to add an extra argument named something like color_by_volume=bool, which is clunkier, but would not force a major version bump. @bdice @vyasr thoughts?

In the meantime, a matplotlib script to achieve the desired color scheme can always be written by hand.

bdice commented 2 years ago

I tend to prefer custom matplotlib for user-defined visualizations. However, I would support the plan to make a PR with API-breaking changes in 3.0 if someone wants to make those changes. @dnusha1994 Would you like to contribute that feature?

When I designed other plotting APIs in freud, I tended towards forwarding *args, **kwargs, which allows passing c=some_array, cmap="viridis" or similar. That's similar to how pandas DataFrame.plot approaches the problem -- just let that logic be handled by matplotlib. However, that only works well for plotting points (with scatter). The patch collections used for Voronoi need more complicated logic to set the colors. See: https://github.com/glotzerlab/freud/blob/f0e81994b2912dbca3e335c1b8f178b4fb9675b9/freud/plot.py#L442-L472

vyasr commented 2 years ago

I agree that we shouldn't overengineer plotting code in freud; the plotting available here should be pretty minimal, just enough for users to validate and get a feel for their analysis. This particular change doesn't seem too heavyweight, though, and I'm fine with adding it. I think the API-breaking change to color_by is probably the way to go since in the interim a sample matplotlib script should suffice to get users through the interim period.

tommy-waltmann commented 1 year ago

addressed in #1012