trevismd / statannotations

add statistical significance annotations on seaborn plots. Further development of statannot, with bugfixes, new features, and a different API.
Other
624 stars 67 forks source link

Printing labels fails if not strings #65

Closed trevismd closed 9 months ago

trevismd commented 2 years ago

Subject of https://stackoverflow.com/questions/72102651/sequence-item-0-expected-str-instance-numpy-int64-found

To fix it, we could cast the struct label to string here.

https://github.com/trevismd/statannotations/blob/bc0c4b8c9565c44427c633c4660ea04b6753292e/statannotations/Annotation.py#L42-L43

JasonMendoza2008 commented 1 year ago

I tried to modify the source code with your proposed solution and it works perfectly (in the case where my categories are integers (e.g. category 0, 1, 2)). Are there some unit tests that get broken when casting the struct label to string? As of right now, fixing the issue usually involves using code that isn't very "clean".

As of right now, when this happens, I usually do: df[column_by] = df[column_by].astype(int).astype(str) where column_by is the column that is going to appear on the x-axis. That obviously only works when my categories are integers.

Although I wouldn't mind trying to make a PR or something, I'm very new when it comes to all of this and would probably need guidance.

trevismd commented 1 year ago

Hello @JasonMendoza2008 , I think we do not have tests with integer/float labels yet. If you want to suggest changes, and we would absolutely appreciate that, you should

  1. Fork the repository
  2. Create a feature branch for your changes based on the dev branch, (like support-non-string-column-names),
  3. Write, commit your changes, and push them to your fork. Ideally, that would include a test making sure that this feature now works.
  4. Open a pull request targeting statannotations:dev, with a reference to this issue.

Please do not hesitate to ask if you'd like to work on it but something is unclear :-)

(Or tell us if you'd prefer someone else to take over.)

JasonMendoza2008 commented 1 year ago

Thanks for the answer. Working on it!

trevismd commented 1 year ago

Hello @JasonMendoza2008, can I help with anything? I'm ready to merge a quick fix with str(struct["label"]) in the snippet above, but I would still appreciate it if you are willing to enhance its robustness with tests, etc. (namely for float values if used/supported at all).

JasonMendoza2008 commented 1 year ago

@trevismd Sorry I kinda forgot I was meant to make a PR. I tried making a test for float values, it does work, although my test is probably not optimal because I re-create a DataFrame and you've already checked if dataframes could be fed through so it might be better to just give sns x=x_axis, y=y_axis with x_axis and y_axis being lists.

Lemme know if I need to change anything!