Skip to content

Conversation

@ausonandres
Copy link
Collaborator

Following issue #41, some functions have been implemented to add plots of the evolution of almost all parameters inside maps.t_evol table. Besides that, all plots are saved as pdf images, instead of being stored as pd.DataFrame in a unique file.

Tests will be added soon.

Copy link
Member

@bpalmeiro bpalmeiro left a comment

Choose a reason for hiding this comment

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

Nice work! Just a few comments, mostly about style. In general, take care of alignment and check new np.array types as np.ndarray

Comment on lines 140 to 144
plt.xticks( rotation=25 )
plt.text(0.03,0.9,
'Mean= {0} \n Std= {1}'.format(np.round(mean,4), np.round(std,4)),
fontsize=10, transform=ax.transAxes,
bbox={'facecolor': 'white', 'alpha': 1, 'pad': 5})
Copy link
Member

Choose a reason for hiding this comment

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

Spacing also inconsistent. Not sure, but maybe @gonzaponte can shed light on the style we follow, but I guess that:

when_in_line(we=don't, add=spaces)
when_not_in_line(we  = do
                 add = spaces)

Copy link
Collaborator

@gonzaponte gonzaponte Mar 12, 2020

Choose a reason for hiding this comment

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

Indeed. But in this case they are combined, so I would give preference to = over =. Maybe even splitting into 1 keyword argument per line.

May I also suggest the use of f-strings and string-formatting syntax?

f'Mean= {mean:.4f} \n Std= {std:.4f}'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, thanks. Regarding your second suggestion we have a question: in the event that you have a variable called, for example: name='Run{0}.pdf', if you want to substitute {0} for a certain value, could it be possible to do it in your way? or only: name.format(runnumber) using format?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the case it is already stored in a variable you need to do .format. f-strings are evaluated immediately, so you either do

  • name = f"Run{run}.pdf" or
  • name = "Run{0}.pdf"; name.format(run)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok! thank you.

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.

3 participants