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

Plotter and CLI fix #49

Closed
wants to merge 29 commits into from
Closed

Plotter and CLI fix #49

wants to merge 29 commits into from

Conversation

lorisercole
Copy link
Member

@lorisercole lorisercole commented Jul 19, 2021

Closes #25, fixes #40, closes #43

Status as of today:

  • The Plotter module has been reorganized. Now it contains all the plot functions.
  • A Plotter subclass can be defined by simply importing the relevant functions.
  • MDSamplePlotter, CurrentPlotter and CLIPlotter have been defined: they contain all the plot functions used by the API and the CLI. The Current class has a _default_plotter attribute (CurrentPlotter).
  • The plot methods are defined dynamically by the set_plotter method. This method takes all the 'plot_*' functions contained in the Plotter object and converts them into methods of Current (or MDSample). This is obtained thanks to a decorator (add_method) that was defined (utils.decorators). The first argument of any plot function (current) becomes self when converted into a class method.
  • The plotter of Current is initialized by default to CurrentPlotter when importing the library. It is possible to change it dynamically by calling the set_plotter method. This is a class method, hence it will change the plotter assigned to the Current class and affect all the current objects already defined.
  • NOTICE: calling set_plotter from a subclass will modify also the plotter of the Current base class. If this behavior is confusing, or not desirable in some applications, we should change it.
  • The base class MDSample uses its own plotter MDSamplePlotter, however it is independent of the Current's plotter chosen.

To Do (in order of importance)

The plotter module has been reorganized (#25).
Now it contains all the plot functions. A `Plotter` subclass can be
defined by simply importing the relevant functions.
`CurrentPlotter` has been defined: it contains all the plot functions
used by `Current`.

The `Current` class has a `_default_plotter` attribute (`CurrentPlotter`).
The plot methods are defined dynamically by the `set_plotter` method.
This method takes all the 'plot_*' functions contained in the `plotter`
input parameter (or the default plotter, e.g. `CurrentPlotter`) and
converts them into methods of `Current`. This is obtained thanks to a
decorator (`add_method`) that was defined. The first argument of the
plot functions (`current`) becomes `self` when converted into a class
method.

The plotter of `Current` is initialized by default to `CurrentPlotter`
when importing the library. It is possible to change it dynamically by
calling the `set_plotter` method. This is a class method, hence it will
change the plotter assigned to the `Current` class and affect all the
current object already defined.
(Notice that calling `set_plotter` from a subclass will modify also the
plotter of the `Current` base class. If this behavior might be
confusing, if it is not wanted in some applications, we should change it.)

It seems that #40 is not the case anymore.

TODO:
- check and remove the commented plot methods from `mdsample` (#43).
- fix the analysis CLI. It is broken now.

In order to close #25 :
- should we define other plotters for the CLI and GUI?
  In case, evaluate shared functionalities.
- how to define plots customizations?
Working on #25.
The function `plotter.style.use_plot_style` sets the plot style of
Matplotlib. Each `Plotter` subclass define its default plot style (e.g.
Current defines 'api_style.mplstyle', the CLI defines
'cli_style.mplstyle', etc.).
New plot styles can be added in the `plotter/styles/` folder, or simply
set via the function mentioned above.
This allows one to define/use different plot style for different uses,
such as the API, CLI, and GUI, if needed.
The `Current.set_plotter` method will call `use_plot_style` when setting
up the plotter.

TODO:
- the styles need to be revised (especially the CLI's)
- A new `MDSamplePlotter` for the `MDSample` base class has been created. The
`set_plotter` class method has been moved into it, and it will be inherited
by `Current`.
- The old plot functions in `mdsample.py` have been removed; `plot_trajectory`
was added to the `plotter.py` module.
- Any reference to `thermocepstrum.utils.plt` & `plotAfterPlt` has been removed.
- `md.resample.resample_timeseries` now calls the `plotter.plot_resample`
method (#43)
- `plotter.plot_resample` now accepts a `MDSample` (sub)-class object and
returns only axes.

TODO:
- fix the `analysis.py` script. There are problems calling the plotter functions
@lorisercole lorisercole self-assigned this Jul 27, 2021
lorisercole and others added 8 commits July 28, 2021 17:34
TODO: check if it is possible to merge CLIPlotter functions with the
API's ones.
Some tests were failing due to old numpy/scipy/pytest versions.
Dependencies updated to numpy>=0.16, scipy>=1.2, pytest>=5.1.

TODO:
 - The imaginary part of the cospectrum is not checked. We should add a
check for complex arrays.
 - Update the examples' reference results.
a factor was missing
@lorisercole lorisercole changed the title Plotter fix Plotter and CLI fix Sep 30, 2021
@rikigigi
Copy link
Member

outdated

@rikigigi rikigigi closed this Dec 15, 2021
@lorisercole lorisercole deleted the plotter_fix branch January 5, 2022 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment