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

add parastell tool, overhaul text wrapping, allow more generalized text in plot #11

Closed
wants to merge 7 commits into from

Conversation

Edgar-21
Copy link
Contributor

plot_parastell_build.py
plot_parastell_build takes a parastell build dict, and phi and theta value, along with the usual plot_radial_build inputs and extracts/formats the data how plot_radial_build likes it, then calls plot_radial_build to make the image and yml file.

plot_radial_build.py
Speaking of, plot_radial_build can now write the yml representation of the plot, and does so by default, along with the usual png.

Additionally, I overhauled how text gets formatted and put into the rectangles, to allow for more flexibility and introduced a 'description' element to the layer dict that allows arbitrary text to be added to each rectangle.

Each layer in the plot_radial_build dict can now have any combination of the three keys, and can also be empty.

General Updates
I added a couple examples as well and updated the readme to indicate they exist, and describe calling plot_radial_build from the command line

Demonstration
Here is the plot generated by the example
image
And here is the plot generated by the parastell tool example
image

closes #10
closes #8 (not notebooks, but I can change that if desired)

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

One high-level design suggestion

Copy link
Member

Choose a reason for hiding this comment

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

What if instead of a different script, this just had a method that extracted a radial build from a parastell build, and then the normal plot_radial_build script could be used?

We shouldn't need to worry about all the plot details in a method that is basically just interpreting a parastell radial build. (separation of concerns)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe went a little over top here, but I combined the functions in plot_radial_build.py into a class, and added a method that interprets parastell builds rather than have a separate script as you suggested. Updated the examples to reflect this change as well. I imagine in the future perhaps the radial build class can be more of its own thing if additional functionality is desired, and just have a method for plotting, rather than having plotting being the main purpose of the class. Sorry for the big diff, almost everything is the same just moved around a bit for use in the class

@Edgar-21
Copy link
Contributor Author

superseded by #13

@Edgar-21 Edgar-21 closed this May 12, 2024
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.

Add some example notebooks to demonstrate Add tool to convert ParaStell radial build at a given theta, phi
2 participants