Skip to content

Implemented wrapper corresponding to tick_params in matplotlib #143

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

Closed
wants to merge 2 commits into from

Conversation

belre
Copy link
Contributor

@belre belre commented Nov 18, 2019

I've been developing VS2017 C++ dll using matplotlib-cpp.
I needed to change some rows of source code in order to adjust whole graph-plotting layouts, especially font size and margin in graph area.

So, I confirmed those in Windows 10 and no problems.
I tested those in static-link(VS2017) and in dynamic-link(called by JVM[JRE 8] ) and used Python 3.6(32bit).

Could you merge those code?
Users will feel more profits in that change.

Copy link
Owner

@lava lava left a comment

Choose a reason for hiding this comment

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

Hey, this looks pretty good, but isn't a

safe_import(s_python_function_tick_params);

missing? Otherwise it looks like the program should crash as soon as the new function is called.

@belre belre requested a review from lava November 19, 2019 23:49
Copy link
Contributor Author

@belre belre left a comment

Choose a reason for hiding this comment

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

Sorry, and I checked my original source code today. Then, I forgot to check omission about 'safe_import'.
I already tested the commit"safe_import is leaked out" before the first commit.

Thanks to point out my failure.

@lava
Copy link
Owner

lava commented Nov 28, 2019

Merged, thanks for contributing!

@lava lava closed this Nov 28, 2019
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.

2 participants