Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve missing value handling logic of ptable_heatmap #241

Closed
wants to merge 7 commits into from

Conversation

DanielYang59
Copy link
Collaborator

@DanielYang59 DanielYang59 commented Nov 4, 2024

Improve missing value handling logic of ptable_heatmap

Our ptable_heatmap has suboptimal missing value handling logic, to close #230:

  • Currently Non-PTableData typed data would be converted, but with the missing_strategy hard-coded as mean, meaning we're artificially creating non-existent data by default also user cannot control this behaviour, perhaps showing missing value as is (-) would be better.
  • Is there a better way to resolve this than simply adding one more argument (on_empty sort of overlaps, perhaps we could partly reuse it)
  • Import performance check show current import time, also round to int (should suffice as import time is on the 1000 level)

TODO list:

  • Make sure a warning would be emitted (only) when data contains NaN/None
  • Test missing value handling for both PTableData and others (pd.DataFrame) when containing missing values (NaN/None)

@DanielYang59 DanielYang59 added matplotlib Concerning matplotlib-powered functions api Application programming interface ptable Periodic table labels Nov 4, 2024
@DanielYang59 DanielYang59 self-assigned this Nov 4, 2024
@janosh
Copy link
Owner

janosh commented Dec 14, 2024

i'll go ahead and close this PR for now as we reached feature parity between the plotly and matplotlib ptable plots and i'm more inclined to maintain the plotly variants due to interactivity, HTML export and ease of testing

@janosh janosh closed this Dec 14, 2024
@DanielYang59
Copy link
Collaborator Author

Thanks for letting me know, fully understandable and I might do back to this a bit later :) Good news is my TODO list for MP stack is mostly cleared :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Application programming interface matplotlib Concerning matplotlib-powered functions ptable Periodic table
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better missing value handling strategy for ptable heatmap
2 participants