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

Remove old plot functions in favor of Plotter methods #43

Closed
3 tasks done
lorisercole opened this issue Nov 11, 2020 · 2 comments · Fixed by #52
Closed
3 tasks done

Remove old plot functions in favor of Plotter methods #43

lorisercole opened this issue Nov 11, 2020 · 2 comments · Fixed by #52

Comments

@lorisercole
Copy link
Member

lorisercole commented Nov 11, 2020

Once the Plotter module is ready (#25), we should remove all traces of the old plot functions. In particular from

  • thermocepstrum/md/mdsample.py
  • thermocepstrum/md/resample.py
  • thermocepstrum/analysis.py
@lorisercole lorisercole added this to the New core API milestone Nov 11, 2020
@lorisercole lorisercole modified the milestones: New core API, Plot engine Jul 8, 2021
lorisercole added a commit that referenced this issue Jul 16, 2021
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.)

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?
lorisercole added a commit that referenced this issue Jul 16, 2021
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?
This was referenced Jul 19, 2021
lorisercole added a commit that referenced this issue Jul 27, 2021
- 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
Copy link
Member Author

Fixed by bb10f3b

@lorisercole
Copy link
Member Author

Closed by #52

@lorisercole lorisercole linked a pull request Dec 20, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant