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

fix: Handle null value for $plotGroupingType in writePlotGroup #3976

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

homersimpsons
Copy link

This is:

  • a bugfix
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

Fixes #3975

@homersimpsons
Copy link
Author

Note that I am not aware enough about the internals to create a concise enough unit test (without relying on an external file), but I would happily use some guidance.

@oleibman
Copy link
Collaborator

Sometimes our tests do need to rely on an external file. Do you have an input file which demonstrates the problem when you load and then try to save it? If so, we can use that to construct a unit test.

@homersimpsons
Copy link
Author

homersimpsons commented Apr 21, 2024

Sometimes our tests do need to rely on an external file. Do you have an input file which demonstrates the problem when you load and then try to save it? If so, we can use that to construct a unit test.

Yes I have such file in #3975: example_plot_grouping_type_null.xlsx

@oleibman
Copy link
Collaborator

Thank you for the sample file. You are correct that your fix eliminates the exception; however, the file that is produced is corrupt (error message when Excel opens it). So your change needs something more. I will take a look, but it may take several days before I can get to it.

@homersimpsons
Copy link
Author

however, the file that is produced is corrupt (error message when Excel opens it).

Okay, so it looks like google sheet has an issue then.

So your change needs something more. I will take a look, but it may take several days before I can get to it.

No worries, I found this bug while investigating another one so I'm not facing it. I put some details in #3975 about my analysis of the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants