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

added: ignore facet args with empty dataset #4038

Merged
merged 8 commits into from
Jun 4, 2023

Conversation

AaronStiff
Copy link
Contributor

@AaronStiff AaronStiff commented Jan 25, 2023

Closes #3984

Code PR

  • I have read through the contributing notes and understand the structure of the package. In particular, if my PR modifies code of plotly.graph_objects, my modifications concern the codegen files and not generated files.
  • I have added tests (if submitting a new feature or correcting a bug) or
    modified existing tests.
  • For a new feature, I have added documentation examples in an existing or
    new tutorial notebook (please see the doc checklist as well).
  • I have added a CHANGELOG entry if fixing/changing/adding anything substantial.
  • For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).

Being brand new to plotly, I'm unsure of where to put a test for this, as the original issue relates to a pandas.DataFrame.

@@ -1988,6 +1988,10 @@ def make_figure(args, constructor, trace_patch=None, layout_patch=None):
layout_patch = layout_patch or {}
apply_default_cascade(args)

# Ignore facet rows and columns when data frame is empty so as to prevent nrows/ncols equaling 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a reasonable fix, but probably belongs near the bottom of the infer_config function, just so it's in roughly the same place as most of the other similar mutations of args.

While we're on this topic of empty data frames though, are there any other args that fail when the df length is 0? would be good to understand why these specifically fail and if it impacts other categorical-type attributes like symbol or somethinng. Maybe there could be a more generic fix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: thanks for the PR! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, that makes sense. And you're very welcome. :)

I've moved it into infer_config for now, but I'll look into whether there could be a more generic solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it looks like facet_col and facet_row both fail with an empty dataset because they are used in the computation of ncols and nrows:

if m.facet == "col":
prefix = get_label(args, args["facet_col"]) + "="
col_labels = [prefix + str(s) for s in sorted_values]
ncols = len(col_labels)
if m.facet == "row":
prefix = get_label(args, args["facet_row"]) + "="
row_labels = [prefix + str(s) for s in sorted_values]
nrows = len(row_labels)

Neither ncols nor nrows can equal 0, hence the failure. As far as I can tell, nothing else in args effects either of the two in any other way.

To me, this seems like a fairly specific failure requiring a specific fix, but I'm still far from familiar with the codebase, so I could easily be wrong. :)

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Thanks @AaronStiff !

@AaronStiff
Copy link
Contributor Author

You're most welcome! 👍

@alexcjohnson alexcjohnson merged commit 3b613fc into plotly:master Jun 4, 2023
@AaronStiff AaronStiff deleted the 3984-facets-empty-dataset branch June 4, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plot empty dataframe fails if facet_col/row is specified
3 participants