fix for chromium renderer on linux systems (issue #2348)

[c-chaitanya]

• [✓ ] 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).

This PR is a fix for issue #2348. I have read the documentation and the contributing notes. Test cases have been added for the same fix. Im not sure if I need to add anything in the changelog. Let me know if anything needs to be changed.

[c-chaitanya]

Fixed black formatting and reopening pull request

[nicolaskruchten]

Thanks for this pull request! I'm rerunning the CI job to see if the orca thing was an isolated failure.

[jonmmease]

Is there a reason to use if any(j in i for j in using) here instead of if i in using here? I think the way it is now, you would get partial string matches, which might be confusing.

Also, I think this will cause an index-out-of-bounds error if none of the browsers in the tuple are available. I don't recall what error is currently raised, but it would be nice to report something a little more informative (e.g. A ValueError that explains that none of the elements in using were found in list(if any(j in i for j in using))).

[c-chaitanya]

Yes @jonmmease using if i in using makes more sense than if any(j in i for j in using), will get that change done.

Regarding the second point I could do something like

if isinstance(using, tuple):
    try:
        using = [i for i in webbrowser._browsers.keys() if i in using][0]
    except TypeError:
         raise TypeError("""
Unable to detect any browsers in webbrowser._browsers.keys().
Check that the desired browser is installed on your system.
If problem still persists, try registering the browser using the python webbrowser module """)

Let me know if the TypeError is easy to understand or any suggestions are welcome

ValueError for invalid names like chromee is already handled and throws and error as follows ValueError: Invalid named renderer(s) received: ['chromee']

[jonmmease]

I think sticking with raising a ValueError makes sense here (to match to existing behavior). And I think you'll need to trap an IndexError rather than a TypeError to handle the case where no matches are found (this is the exception raised when you evaluate [][0]).

For the error message, I pictured including the actual list of supported browsers (the result of evaluating list(webbrowser._browsers.keys())) rather than "webbrowser._browsers.keys()") rather than itself.

Thanks!

[c-chaitanya]

Hi @jonmmease can you please help me out with the content to put in the value error. Does this content seem ok? Any suggestions are welcome.

if isinstance(using, tuple):
    try:
        using = [i for i in webbrowser._browsers.keys() if i in using)][0]
    except IndexError:
        raise ValueError("""
Unable to detect the given browser.
Try one among the following "chrome", "chromium", "firefox" or "default" to use your default browser.
""")

[jonmmease]

Thanks a lot @c-chaitanya. I left one small comment. It'll be great to get this in!

[nicolaskruchten]

@c-chaitanya if you can address Jon's last comment I can try to get this one into the next release next week!

[c-chaitanya]

Hi @nicolaskruchten, I have addressed @jonmmease comments on the pull request. The test cases seem to fail for chart_studio while my changes are isolated toward plotly packages. I do not have permissions to rerun the circleci workflow. Let me know if anything is pending from my side. Ill try to get it fixed asap for the new release.

[jonmmease]

Thanks @c-chaitanya. lgtm, but will let @nicolaskruchten do the merge

[nicolaskruchten]

Thanks very much!