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

Automatically adjust y-limits and y-ticks of expression plots #55

Merged
merged 6 commits into from
Nov 16, 2023

Conversation

caiw
Copy link
Member

@caiw caiw commented Nov 10, 2023

Fixed #51.

caiw added 2 commits November 10, 2023 21:22
…ults)

yticks are spaced at intervals of 10^-50, no matter the size of the data
@caiw caiw added ready-for-merging 📈 plotting functionality Any issues related to plotting labels Nov 10, 2023
@caiw caiw requested a review from neukym November 10, 2023 21:25
@caiw caiw self-assigned this Nov 10, 2023
@caiw caiw linked an issue Nov 10, 2023 that may be closed by this pull request
@neukym
Copy link
Member

neukym commented Nov 11, 2023

OK - First of all I love the auto-rescaling when no argument is given. 👍

My only nitpick is this doesn't sort the problem when ylim is set by the user as (say) 10 ** -172. All the ticks will be blank. But I think _get_yticks() can be tweaked to get this -

def _get_yticks(ylim_oom):
    order_of_magnitude = int(np.floor(np.log10(ylim_oom) / _OOM_SIZE)) * -1
    ylim_oom = 10 ** - ((order_of_magnitude -1) * _OOM_SIZE)
    return np.geomspace(start=1, stop=ylim_oom, num=order_of_magnitude + 1)

This will then correctly provide the ticks spaced out by 10 ** -50, but the ylim (set elsewhere) will add some extra y-axis on top if needed to get to the users requested value ylim. This extra space will not have any minor ticks (so there will be no tick with 10 ** -172 for instance), but I don't think there needs to be - this is the expected behaviour, in my mind at least.

kymata/plot/plotting.py Outdated Show resolved Hide resolved
@caiw
Copy link
Member Author

caiw commented Nov 16, 2023

My only nitpick is this doesn't sort the problem when ylim is set by the user as (say) 10 ** -172. All the ticks will be blank.

Ooh, not a nitpick, a good catch! Thanks for the fix. Happy for me to merge it in after?

@neukym
Copy link
Member

neukym commented Nov 16, 2023

Yes please!

caiw added 3 commits November 16, 2023 14:18
…0^{-50}

Also rename "order of magnitude" to something more technically accurate.
@caiw caiw merged commit bee002e into main Nov 16, 2023
1 check passed
@caiw caiw deleted the final-adjustments-to-plot_expression_plot-ylim branch November 16, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 plotting functionality Any issues related to plotting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Final adjustments to plot_expression_plot(..., ylim)
2 participants