Changes in functions for generation and an example of vertical plot with additional charts #182

ennanco · 2022-02-24T18:47:56Z

I have slightly modified the functions generate_samples and generate_counts to get the data to develop a vertical Upsetplot with additional charts attached to it.

…nerate more than one feature

jnothman

Any chance you feel like extending this to close #52 at the same time? (I.e. add some tests for these functions)

jnothman · 2022-02-28T00:21:49Z

And thank you for contributing!

ennanco · 2022-02-28T08:34:54Z

Thank you for the library you have done the heavy lifting in this point. I am not used to writing automatic tests on python But I could sure try to do it. Sorry for the long delay on the example but as you know it has been a long and hard time the last year.

…

On Mon, Feb 28, 2022 at 1:21 AM Joel Nothman ***@***.***> wrote: And thank you for contributing! — Reply to this email directly, view it on GitHub <#182 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABWRIKRPIOXE45HCPFU7PO3U5K52PANCNFSM5PIGFHXA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

ennanco · 2022-02-28T19:14:35Z

Already added the unitary tests for those two functions. You should review them but I think that you can close issue #52

…amples

ennanco · 2022-03-02T13:34:17Z

I have made some additional changes, but some examples failed due to their usage of the property name which, by the way, can cause like in this case problems when we change from a single column view to a multi-column result

jnothman · 2022-03-02T23:47:05Z

With luck, I'll have time to look at this next week, @ennanco

jnothman · 2022-03-19T12:42:14Z

The failures currently in CI pertain to our continued support for very old version of Python (time to drop them, I think), and PEP8 violations in your contribution. Here are those PEP8 issues:

./upsetplot/data.py:40:31: E228 missing whitespace around modulo operator
./upsetplot/data.py:40:40: E225 missing whitespace around operator
./upsetplot/data.py:40:80: E501 line too long (82 > 79 characters)
./upsetplot/data.py:45:19: E228 missing whitespace around modulo operator
./upsetplot/data.py:45:28: E231 missing whitespace after ','
./upsetplot/data.py:49:26: E228 missing whitespace around modulo operator
./upsetplot/tests/test_data.py:210:1: E302 expected 2 blank lines, found 1
./upsetplot/tests/test_data.py:228:8: E111 indentation is not a multiple of 4
./upsetplot/tests/test_data.py:232:8: E111 indentation is not a multiple of 4
./upsetplot/tests/test_data.py:233:8: E111 indentation is not a multiple of 4
./upsetplot/tests/test_data.py:233:34: E231 missing whitespace after ','
./upsetplot/tests/test_data.py:234:8: E111 indentation is not a multiple of 4
./upsetplot/tests/test_data.py:237:48: E231 missing whitespace after ','
./upsetplot/tests/test_data.py:238:47: E231 missing whitespace after ','
./upsetplot/tests/test_data.py:239:53: E231 missing whitespace after ','
./upsetplot/tests/test_data.py:239:80: E501 line too long (80 > 79 characters)
./upsetplot/tests/test_data.py:243:80: E501 line too long (84 > 79 characters)
./upsetplot/tests/test_data.py:255:55: E226 missing whitespace around arithmetic operator
./upsetplot/tests/test_data.py:258:47: E231 missing whitespace after ','
./upsetplot/tests/test_data.py:259:29: E211 whitespace before '('
./upsetplot/tests/test_data.py:261:80: E501 line too long (86 > 79 characters)
./upsetplot/tests/test_data.py:267:1: W391 blank line at end of file
./examples/plot_vertical.py:31:80: E501 line too long (82 > 79 characters)
./examples/plot_vertical.py:33:80: E501 line too long (106 > 79 characters)
./examples/plot_vertical.py:39:1: W391 blank line at end of file

I could just adopt black to make resolving these simpler...

jnothman · 2022-03-19T12:46:37Z

Thanks for your efforts here! Can we make a point of keeping the existing behaviour of generate_counts stable and backwards compatible? That is, unless extra columns are requested, the user should not have to go .value. Should we call the new generation parameter extra_columns?

ennanco · 2022-03-19T16:35:47Z

Hi Joel, I think that the proposed name would be suitable in order to keep it as retrocompatible as possible. Sincerely,

…

----- Quique

On Sat, Mar 19, 2022 at 1:46 PM Joel Nothman ***@***.***> wrote: Thanks for your efforts here! Can we make a point of keeping the existing behaviour of generate_counts stable and backwards compatible? That is, unless extra columns are requested, the user should not have to go .value. Should we call the new generation parameter extra_columns? — Reply to this email directly, view it on GitHub <#182 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABWRIKUTHVVX3EROVNQUYGTVAXEDRANCNFSM5PIGFHXA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

ennanco · 2022-03-19T19:11:01Z

I have finally make the proposed changes with the extra_columns parameter instead of len_sample in generate_counts

jnothman

Can we do the same in generate_samples? Could you please update the docstring? Thanks

…consistency to use extra_columns in order to keep retrocompatibility

ennanco · 2022-03-22T08:38:35Z

Yes, I thought about it, however, I don't want to change it without your indication. Sorry about the docstring, I have already updated it.

jnothman

Finally reviewing this! It's looking pretty good, but I'm wondering about:

the use of the name value{i}, or whether there's a distinctly better column name
whether we should consider generating values in a way that varies with the categories, such that the visualisations show covariance

upsetplot/data.py

examples/plot_vertical.py

Co-authored-by: Joel Nothman <joeln@canva.com>

jnothman · 2023-01-02T12:24:19Z

upsetplot/data.py

-    df = pd.DataFrame({'value': np.zeros(n_samples)})
+    len_samples = 1 + extra_columns
+    df = pd.DataFrame(np.zeros((n_samples, len_samples)))
+    valuename_lst = [f'value{i}' if i > 0 else 'value' for i in


Can we just call this variable columns or column_names?

jnothman · 2023-01-02T12:26:21Z

upsetplot/data.py

+                          extra_columns=extra_columns)
+    df.drop('index', axis=1, inplace=True)
+    df = df if extra_columns > 0 else df.value
+    return df.groupby(level=list(range(n_categories))).count()


I don't think counting is meaningful for the extra columns. Maybe we should use a different aggregate?

Or maybe we shouldn't offer this functionality in generate_counts, making things somewhat simpler.

jnothman · 2023-01-02T12:30:01Z

upsetplot/data.py

+        r = rng.rand(n_samples, len_samples)
+        df[f'cat{i}'] = r[:, 0] > rng.rand()


This puzzles me. We're only using the first column of a random matrix of values, and extra_columns is unused.

Don't worry about making the values correlate with the categories. Just put in the docstring that the extra column values may change in a future version so we have licence to do it later.

jnothman · 2023-01-02T12:46:53Z

examples/plot_vertical.py

+#########################################################################
+# An UpSetplot with additional plots on vertical
+# and tuning some visual parameters
+example = generate_counts(extra_columns=2)


I think using generate_samples here makes more sense? But maybe 10k samples is a lot for three swam plots.

changes in functions generate_samples and generate_counts to allow ge…

83a15bb

…nerate more than one feature

jnothman reviewed Feb 28, 2022

View reviewed changes

added unitary tests for generate_samples and generate_counts funtions

5dab608

ennanco added 2 commits March 2, 2022 14:07

Repaired problems with some tests

3eb9c65

Repaired several examples due to the inclussion of the new generate_s…

e40d6b9

…amples

ennanco added 2 commits March 2, 2022 18:16

Change the string format to make it compatible with Python v2

7f5f918

Adding compatibility in generate_samples for python v2

e0d9df0

Adding adaptations to made it retrocompatible with the examples

4e18668

ennanco added 4 commits March 19, 2022 21:30

Fixing style

b0c9c7b

Fixing test_data.py according to python style sheet

3374d0d

Fixing indentation

9c546e0

Fixing indentation

7805d3f

jnothman reviewed Mar 21, 2022

View reviewed changes

Fixing doctring in generete_counts and changing generate_samples for …

4cab536

…consistency to use extra_columns in order to keep retrocompatibility

ennanco and others added 5 commits March 22, 2022 09:43

Fixing spacing style in some comments

b1d0ec6

Adding unitary test for generate_data

f64fb19

Adding unitary test for generate_data

5443acb

Adding unitary test for generate_data

5731bc6

Merge branch 'jnothman:master' into master

ede49b5

jnothman reviewed Jan 1, 2023

View reviewed changes

upsetplot/data.py Outdated Show resolved Hide resolved

upsetplot/data.py Outdated Show resolved Hide resolved

upsetplot/data.py Outdated Show resolved Hide resolved

upsetplot/data.py Outdated Show resolved Hide resolved

examples/plot_vertical.py Outdated Show resolved Hide resolved

ennanco and others added 5 commits January 2, 2023 10:01

Update upsetplot/data.py

062e337

Co-authored-by: Joel Nothman <joeln@canva.com>

Update upsetplot/data.py

3d884c4

Co-authored-by: Joel Nothman <joeln@canva.com>

Update upsetplot/data.py

b000f15

Co-authored-by: Joel Nothman <joeln@canva.com>

Update upsetplot/data.py

684be8c

Co-authored-by: Joel Nothman <joeln@canva.com>

Update examples/plot_vertical.py

ce55bd0

Co-authored-by: Joel Nothman <joeln@canva.com>

jnothman reviewed Jan 2, 2023

View reviewed changes

ennanco added 2 commits January 3, 2023 16:27

Merge branch 'jnothman:master' into master

35ef9bf

Merge branch 'jnothman:master' into master

746f679

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes in functions for generation and an example of vertical plot with additional charts #182

Changes in functions for generation and an example of vertical plot with additional charts #182

ennanco commented Feb 24, 2022

jnothman left a comment

jnothman commented Feb 28, 2022

ennanco commented Feb 28, 2022 via email

ennanco commented Feb 28, 2022

ennanco commented Mar 2, 2022

jnothman commented Mar 2, 2022

jnothman commented Mar 19, 2022

jnothman commented Mar 19, 2022

ennanco commented Mar 19, 2022 via email

ennanco commented Mar 19, 2022

jnothman left a comment

ennanco commented Mar 22, 2022

jnothman left a comment

jnothman Jan 2, 2023

jnothman Jan 2, 2023

jnothman Jan 2, 2023

jnothman Jan 2, 2023

jnothman Jan 2, 2023

jnothman Jan 2, 2023

		r = rng.rand(n_samples, len_samples)
		df[f'cat{i}'] = r[:, 0] > rng.rand()

Changes in functions for generation and an example of vertical plot with additional charts #182

Are you sure you want to change the base?

Changes in functions for generation and an example of vertical plot with additional charts #182

Conversation

ennanco commented Feb 24, 2022

jnothman left a comment

Choose a reason for hiding this comment

jnothman commented Feb 28, 2022

ennanco commented Feb 28, 2022 via email

ennanco commented Feb 28, 2022

ennanco commented Mar 2, 2022

jnothman commented Mar 2, 2022

jnothman commented Mar 19, 2022

jnothman commented Mar 19, 2022

ennanco commented Mar 19, 2022 via email

ennanco commented Mar 19, 2022

jnothman left a comment

Choose a reason for hiding this comment

ennanco commented Mar 22, 2022

jnothman left a comment

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment

jnothman Jan 2, 2023

Choose a reason for hiding this comment