2952 addition of revised radial build plotting tool #2955
Conversation
process/io/plot_radial_build.py
Outdated
| "-ib", | ||
| "--inboard", | ||
| action="store_true", | ||
| help="Show inboard build only, default = No ", |
There was a problem hiding this comment.
default = False as per Python conventions
There was a problem hiding this comment.
I hadn't noticed this, what you have done is correct... I meant change 'default = No' to 'default = False'
process/io/plot_radial_build.py
Outdated
| if ifail[ii] == 1: | ||
| for jj in range(len(radial_labels)): | ||
| radial_build[jj, ll] = m_file.data[radial_labels[jj]].get_scan(ii + 1) | ||
| ll += 1 |
There was a problem hiding this comment.
I think this could be simplified so that there is an array that we append only converged solutions to, and then turn into a numpy array. This was we dont need to count the number of converged solutions and we don't need to do this ugly loop. Let me know if this does not make sense.
process/io/plot_radial_build.py
Outdated
| # This needs to be kept in sync automatically; this will break frequently | ||
| # otherwise | ||
| # Rem : Some variables are not in the MFILE, making the defintion rather tricky... | ||
| nsweep_dict = dict() |
There was a problem hiding this comment.
Appears to be creating a dict where the keys are integers (aka an array), why?
There was a problem hiding this comment.
This is copied directly from plot_scan.py and is code originally written by Seb. There is definitely scope to tidy this up. The integer here refers to the scan variable nsweep.
There was a problem hiding this comment.
Could you change it to be an array? This will be more idiomatic maybe probably shorter
| except Exception: | ||
| scan_var_name = "Null" | ||
|
|
||
| radial_labels = [ |
There was a problem hiding this comment.
Is this a duplicate of the above variable (albeit in a different scope)? If so, could you make it a constant defined at the top (after the imports) of the file.
You could also consider if we could consolidate this information and the LaTex labels together.
There was a problem hiding this comment.
This is in to allow plotting for scanning MFILE's and non scan MFILE's. There is an attempt to get the sweep variable and if its not able to it throws an exception and sets the y-axis to 0
There was a problem hiding this comment.
My point is I think we define the exact same list twice, and it could just be defined once
| `plot_radial_build` is to plot the radial build of the machine in terms of bar segments. It can be run as follows: | ||
|
|
||
| ```bash | ||
| python process/io/plot_radial_build.py -f path/to/MFILE.DAT |
There was a problem hiding this comment.
Should possibly show how the -o flag could be used here too. The others are a bit too obscure to document here.
timothy-nunn
deleted the
2952-addition-of-revised-radial-build-plotting-tool
branch
* Initial file commit * bare except flake 8 fix * Initial no-scan plot attempt * Integration test and outputdir command option * remove outputdir * integration test save directory fix * Documentation additions * Added in plot_proc images as extra * default false argparse * added -o flag to docs * default false * ifail sweep tidy up * dict to list * file naming added
Description
The radial build plotting tool initially created by @ajpearcey has now been revised.
test_plot_radial_build.pytf_in_csswitchChecklist
I confirm that I have completed the following checks: