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

Call to atomman differential_displacement broken by new version #19

Open
arn-all opened this issue Apr 25, 2019 · 1 comment
Open

Call to atomman differential_displacement broken by new version #19

arn-all opened this issue Apr 25, 2019 · 1 comment

Comments

@arn-all
Copy link
Contributor

arn-all commented Apr 25, 2019

Hi,

Error

In matscipy.dislocation.plot_vitek(), the call to the atomman package function differential_displacement provokes the following error :

TypeError: differential_displacement() got an unexpected keyword argument 'show'
with atomman version >1.2.4.

Piece of code involved

Below, the call to atomman in matscipy :

am.defect.differential_displacement(base_system, disl_system,
                                        burgers,
                                        cutoff=neighborListCutoff,
                                        xlim=plot_range[0],
                                        ylim=plot_range[1],
                                        zlim=plot_range[2],
                                        plot_scale=plot_scale,
                                        show=show, save_file=save_file,
plot_axes=plot_axes)

Reason

It is because we changed the arguments passed to the function to make it more general (see here).

Proposed solution

The fix needs to account that differential_displacement now returns a matplotlib figure object, that can be saved later directly in plot_vitek.
When plot_vitek is called in dislocation.show_NEB_configurations(), there is a little change to make to be able to add new vitek plots on top of each other.

In the meantime, one can still use versions of atomman that are <=1.2.3.

@pgrigorev
Copy link
Contributor

Hi,
Thanks for reporting that. I was using an old version and discovered this change only recently.
I have just noticed that something weird happened with this commit:
usnistgov/atomman@7de5f63#diff-7413d2aec84a53464dc3c1440a13b8e3

Basically it wiped out the changes of my pull request where I aded possibility of passing matplotlib axes to the function via plot_axes argument.

Was it actually done by purpose? If not it would be grate if you can reverse it. Alternatively I can reimplement it and to another pull request.

This is more preferable that a returning a figure since one can imagine having a figure with multiple axes. When you force to create a new figure on every function call you limit the usage of the function to figures with single axes.

Having control over you figure and axes outside the plotting function also would allow you to add impurities and any extra data and makes the function much more versatile. This is also a standard behaviour for plotting functions in the modules built on top of the matplotlib. See seabron as an example: https://github.com/mwaskom/seaborn

Petr

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

No branches or pull requests

2 participants