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

Remove MakieLayout module #2008

Merged
merged 10 commits into from
Jun 6, 2022
Merged

Remove MakieLayout module #2008

merged 10 commits into from
Jun 6, 2022

Conversation

SimonDanisch
Copy link
Member

@SimonDanisch SimonDanisch commented May 31, 2022

Pretty much the same as #2007 just for MakieLayout.
The extra module has just been leading to repeating code without real benefits, and we have been acting like Makie is the sole API exporter anyways.
This also removes the extra __init__ and combines the precompiles and adds some new ones.
Should also not be breaking due to @deprecate_binding, although the question remains when to remove the @deprecate_binding

@MakieBot
Copy link
Collaborator

MakieBot commented May 31, 2022

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt.

using time

master:  10.96 < 11.02 > 11.08, 0.04+-
pr:      10.90 < 11.03 > 11.07, 0.06+-
speedup: 1.00 < 1.00 > 1.01, 0.00+-
median:  +0.11% => invariant

This PR does not change the using time.

ttfp time

master   31.14 < 31.33 > 31.46, 0.11+-
pr       31.04 < 31.19 > 31.38, 0.13+-
speedup: 1.00 < 1.00 > 1.01, 0.00+-
median:  -0.42% => invariant

This PR does not change the ttfp time.

Copy link
Member

@jkrumbiegel jkrumbiegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you didn't check the docs for MakieLayout occurrences right? I know there's a couple mentions on the Axis page at least, so it would be good to also remove those

@SimonDanisch SimonDanisch merged commit 863a46f into master Jun 6, 2022
@SimonDanisch SimonDanisch deleted the sd/remove-makielayout branch June 6, 2022 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants