Make FuncFormatter work without FixedLocator. #69
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is impossible for exported plot to ever call the python function, so we need to decide what to do when the user uses a FuncFormatter. I decide that it's most meaningful to convert it to "fixed" ticks at export-time, which is what this code does. However, it still keeps the type as "func" so the consumer of export may choose what to do. mpld3 just treats "func" and "fixed" identically. We could argue about changing to "fixed" here at export-time; I think keeping "func" conveys more information, but I don't feel strongly about it. The current state for FuncFormatter is that it does convert the labels at export-time only if locator is fixed. I argue this is strictly worse. At the same time, I also made the unit-test assert util print much more human interpretable test failure outputs, see comment.
Author
|
ping |
Member
|
Have been sick, but will take a look at this ASAP! |
Author
|
oh no rush sorry, get well first! |
Member
|
Should be able to get to this next week, thanks for waiting! 🙏🏻 |
Author
|
no rush, I only pinged to avoid it getting forgotten, not to pressure! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is impossible for exported plot to ever call the python function, so we need to decide what to do when the user uses a FuncFormatter.
I decide that it's most meaningful to convert it to "fixed" ticks at export-time, which is what this code does. However, it still keeps the type as "func" so the consumer of export may choose what to do.
mpld3 just treats "func" and "fixed" identically. We could argue about changing to "fixed" here at export-time; I think keeping "func" conveys more information, but I don't feel strongly about it.
The current state for FuncFormatter is that it does convert the labels at export-time only if locator is fixed. I argue this is strictly worse.
At the same time, I also made the unit-test assert util print much more human interpretable test failure outputs, see comment.
PS: I used gpt5-codex-high to help me locate and understand the issue, but I implemented the fix myself.