Fix usage of column_names which is part of Interchange protocol

[anmyachev]

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).

[anmyachev]

Hi @MarcoGorelli! Could you take a look?

[MarcoGorelli]

looks fine

[anmyachev]

looks fine

@MarcoGorelli thanks for the feedback!

[anmyachev]

Hi @alexcjohnson! I'm not sure if it's necessary to add tests here, do you think it's worth adding? The change looks safe enough.

For example, I could add a synthetic test where the generator from this function will be returned instead of list.

plotly.py/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py

Line 261 in 63b9ac5

return self._columns

[alexcjohnson]

I see, so in principle column_names could be a non-materialized list - generator, iterator, whatever, but we actually depend on columns being materialized, indexable etc... and so far all the dataframes we've encountered have it materialized but in principle that could change any time and still be compliant with the protocol. Do I have that right?

If so, then yes, I think your proposed test would be great. Given that this PR is fixing a case we hadn't previously covered it definitely wants a test one way or another, that would have failed without this change. After that the only other thing we'll want is a changelog entry.

Thanks for the PR!

[anmyachev]

I see, so in principle column_names could be a non-materialized list - generator, iterator, whatever, but we actually depend on columns being materialized, indexable etc... and so far all the dataframes we've encountered have it materialized but in principle that could change any time and still be compliant with the protocol. Do I have that right?

Yes, you are right. Thanks for the quick response!

If so, then yes, I think your proposed test would be great. Given that this PR is fixing a case we hadn't previously covered it definitely wants a test one way or another, that would have failed without this change. After that the only other thing we'll want is a changelog entry.

I added a test and a log entry.

[anmyachev]

This change is necessary so that this attribute is not saved between runs.

[alexcjohnson]

💃 Very nicely done @anmyachev - thanks again!