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

Shorter python imports for postprocessing tools #2727

Merged

Conversation

codingS3b
Copy link
Contributor

@codingS3b codingS3b commented Oct 15, 2018

In order to shorten the import statements when using python visualizers, they are now included within the plot_mpl/__init__.py with meaningful aliases (since they are all called Visualizer).

Instead of writing
from picongpu.plugins.plot_mpl.phase_space_visualizer import Visualizer as PhaseSpaceVisualizer one could then shorten this to from picongpu.plugins.plot_mpl import PhaseSpaceVisualizer

@codingS3b codingS3b changed the title Add import aliases to plot_mpl init Shorter python imports for postprocessing tools Oct 15, 2018

__all__ = ["EnergyHistogram",
"PhaseSpace",
"PNG"]
Copy link
Member

Choose a reason for hiding this comment

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

missing EOF

@ax3l ax3l self-assigned this Oct 15, 2018
@ax3l ax3l self-requested a review October 15, 2018 11:49
@ax3l ax3l added the component: tools scripts, python libs and CMake label Oct 15, 2018
@ax3l ax3l added this to the 0.5.0 / 1.0.0: Next Stable milestone Oct 15, 2018
from .png_visualizer import Visualizer as PNGVisualizer


__all__ = ["EnergyHistogramVisualizer",
Copy link
Member

@ax3l ax3l Oct 15, 2018

Choose a reason for hiding this comment

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

I would not suffix them with Visualizer, although I see the charm when importing data readers and visualizers at the same time into main scope... Do you know any best practices on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, not really. I would also have to read up on that.
Naming collisions were the main reason for suffixing them this way.

Copy link
Member

@ax3l ax3l Oct 15, 2018

Choose a reason for hiding this comment

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

Ok, but then let us at least Suffix them with PlotMPL or just MPL to be descriptive and consistent with the dir name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I changed the suffix to 'MPL'

@ax3l ax3l merged commit 36eabde into ComputationalRadiationPhysics:dev Oct 15, 2018
@ax3l ax3l mentioned this pull request Oct 15, 2018
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tools scripts, python libs and CMake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants