plotly / plotly.py

The interactive graphing library for Python :sparkles: This project now includes Plotly Express!
https://plotly.com/python/
MIT License
16.06k stars 2.54k forks source link

Handle `extras` option in `IntegerValidator` #4612

Closed archmoj closed 2 months ago

archmoj commented 4 months ago

extras are added to plotly.js integer validator in https://github.com/plotly/plotly.js/pull/6990/commits/37dee4852e335067508add73de7ee7a59e8db045 i.e. to handle cases like bold and normal in font.weight which could also be a number between 1-1000.

This commit would be required in updating to upcoming plotly.js v2.33.0.

@LiamConnors cc: @emilykl @gvwilson

archmoj commented 3 months ago

@LiamConnors please merge this PR once you added a changelog regarding the changes, etc. as required. Thank you very much in advance :pray:

emilykl commented 3 months ago

Logic looks good to me. Thanks @archmoj !

Do we have tests for the validators? Can we add a test for this change?

archmoj commented 3 months ago

Logic looks good to me. Thanks @archmoj !

Do we have tests for the validators? Can we add a test for this change?

Good call. It would be nice to add tests for this PR. @LiamConnors you may add a test in packages/python/plotly/_plotly_utils/tests/validators/test_dash_validator.py

Thank you!

archmoj commented 2 months ago

@LiamConnors The tests used to pass before your commit which simply added a line to the changelog. But now it fails. Do you have any idea why this is happening?

LiamConnors commented 2 months ago

@archmoj if you update from master to bring in this change https://github.com/plotly/plotly.py/pull/4625 it should fix it

archmoj commented 2 months ago

I'll add tests in a separate PR later.