konstantint / matplotlib-venn

Area-weighted venn-diagrams for Python/matplotlib
MIT License
520 stars 68 forks source link

added optional radius param for venn2 #74

Closed Irfan-Ahmad-byte closed 1 year ago

Irfan-Ahmad-byte commented 1 year ago

i added optional radii parameter to venn2 function, to customize radius of venn diagrams

konstantint commented 1 year ago

Hi, thanks for the PR.

I don't think this is a useful argument to have for the function, though. The radii are one-to-one derivable from set sizes (and the normalize_to) arguments (and vice-versa). Thus, specifying both in the argument list adds unnecessary confusion to the function.

Things are made more confusing by the fact that you to compute centers based on the subset sizes while changing the radii to some other values, which makes the outcome uninterpretable.

Maybe if you clarify your particular use-case we could come up with a solution for it that does not require changing the code?

Irfan-Ahmad-byte commented 1 year ago

Thanks for your consideration @konstantint

My use case: I was working with some biological data with different sizes, huge difference (after cleaning etc). In this case 1 of the circles in the Venn had very small radii to display in a project. So I managed set a radii parameter so that in case there is huge difference in datasets, we still be able to visualize it without losing its size.

It worked for me, hence I thought that others may also face problem so we can place there a parameter to pre-define the redii. You can see that in case one specifies custom radii, radii are not calculated with centers, but centers are still calculated because they should be automatically calculated to prevent user's headache.

why I think it should be included in the package:

And, currently I have added it for Venn2 only to see if this will be a valuable feature.

konstantint commented 1 year ago

.. but the problem is solved trivially by hand-tuning the set sizes instead of hand-tuning the radii.

E.g. if you have original set sizes of A/B=10000, B/A=1, A&B=1, then nothing prevents you from scaling up the two latter values to whatever you consider visually more appropriate, e.g. by calling the method with values 10000, 1000, 1000 instead. There is even a special "venn2_unweighted" method for this specific use-case.

This would achieve qualitatively the same effect as hardcoding the radii (you will make the B circle larger), while at the same time you will have full control and awareness of how much exactly you are misleading the viewer in the displayed areas.

On the other hand, setting the radii is quite arbitrary and uninterpretable in terms of data, which is opposing the main point of the package (that of visualizing data).

To make things worse, making arbitrary changes to the radii may leave you with circles that the package cannot even visualize (e.g. if you make radius1 = very large and radius2 = very small, the resulting diagram will have one circle fully surrounded by another, and there is no logic in it to depict such regions (it would just throw an exception).

Irfan-Ahmad-byte commented 1 year ago

Yeah, you've got a point bro. I thought I was making some contribution.

Thanks for considering and guiding.