doc/tutorial: adding a tutorial for MorphFuncy and proper docstring - #215
sbillinge merged 5 commits into
Conversation
Sorry, something went wrong.
sbillinge
left a comment
There was a problem hiding this comment.
nicely done. Please see comments.
Sorry, something went wrong.
| ``MorphFuncy`` requires a Python function and is therefore intended to be used | ||
| within the Python API. | ||
|
|
||
| 1. Import the necessary modules into your Python script :: |
There was a problem hiding this comment.
I think this is copied from the other docs but it is not the preferred pattern. At least for new docs (not sure if we want to fix all the old code blocks, but maybe?) It looks better if:
1. Import the necessary modules into your Python script ::
from diffpy.morph.morph_api import morph, morph_default_config
import numpy as np
goes to
1. Import the necessary modules into your Python script
.. code-block:: python
from diffpy.morph.morph_api import morph, morph_default_config
import numpy as np
I am not 100% sure about the indentation of the .. code-block and there are requirements for blank lines here and thee, but you can figure it out so it builds correctly it will be color coded in the built documentation.
Sorry, something went wrong.
|
|
||
| 2. Define a custom Python function to apply a transformation to the data. | ||
| For this example, we will use a simple linear transformation that | ||
| scales the input and applies an offset :: |
There was a problem hiding this comment.
maybe explicitly specify requirements: x and y must be vectors of the same length, parameters are passed in, transformed x and y vectors are returned...etc. I am making this up, but having this in the docs is important I think.
Sorry, something went wrong.
|
|
||
| cfg = morph_default_config(funcy={"scale": 1.2, "offset": 0.1}) | ||
| cfg["function"] = linear_function | ||
|
|
There was a problem hiding this comment.
I would add a line that shows what the dictionary looks like after it is created in case users want to create it in an editor and not use python code.
Sorry, something went wrong.
There was a problem hiding this comment.
@Luiskitsu gentle ping on this
Sorry, something went wrong.
| cfg = morph_default_config(funcy={"scale": 1.2, "offset": 0.1}) | ||
| cfg["function"] = linear_function | ||
|
|
||
| 5. Run the morph using the API function ``morph(...)``. This will apply the |
There was a problem hiding this comment.
I would avoid using Computer science jargon such as "API function" I think it is more accessible to people if you just tell them what to do and show an example. It really doesn't matter if morph() as an API function or some other function. For example, math.sin() is an API function but I never see documentation that says "calculate the sine using the API function sin()"
Sorry, something went wrong.
| x_morph_out, y_morph_out, x_target_out, y_target_out = morph_rv["morph_chain"].xyallout | ||
|
|
||
| fitted_parameters = morphed_cfg["funcy"] | ||
| print("Fitted scale:", fitted_parameters["scale"]) |
There was a problem hiding this comment.
let's model using f-strings here for the print
Sorry, something went wrong.
| user-defined function and refine the parameters to best align the morph data | ||
| with the target data :: | ||
|
|
||
| morph_rv = morph(x_morph, y_morph, x_target, y_target, **cfg) |
There was a problem hiding this comment.
what is rv? again, longer variable names that are more descriptive here are probably better.
Sorry, something went wrong.
| transformation parameters (our initial guess) and the transformation | ||
| function itself :: | ||
|
|
||
| cfg = morph_default_config(funcy={"scale": 1.2, "offset": 0.1}) |
There was a problem hiding this comment.
can we model the use of better variable names?
Sorry, something went wrong.
| class MorphFuncy(Morph): | ||
| """Apply the user-supplied Python function to the y-coordinates of the | ||
| morph data""" | ||
| """General morph function that applies a user-supplied function to the |
There was a problem hiding this comment.
This needs one high level statement of <=72 characters, then blank line, then a longer description
Sorry, something went wrong.
|
|
||
| parameters: dict | ||
| A dictionary of parameters to pass to the function. | ||
| These parameters are unpacked using **kwargs. |
There was a problem hiding this comment.
This line not needed. User doesn't care how the program works, just how to use it.
Sorry, something went wrong.
|
@Luiskitsu this is conflicted. I think it is just a reformatting of a docstring, so you should be able to just accept the remote changes in the diff but check you don't lose any hand edited stuff there. |
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files @@ Coverage Diff @@
## main #215 +/- ##
=======================================
Coverage ? 99.04%
=======================================
Files ? 21
Lines ? 1048
Branches ? 0
=======================================
Hits ? 1038
Misses ? 10
Partials ? 0
|
Sorry, something went wrong.
Hi Simon, I have added a tutorial for the MorphFuncy. Note that MorphFuncy has to be used with the API. For the MorphSqueeze Andrew and I have thought might be better to first add it to the CLI and then do a tutorial similar to the current one where it uses all the different morphs together.
I have also changed the docstrings for MorphFuncy and MorphSqueeze to be consistent with the other morphs. In order to see the changes and how it looks in I think needs to be merged first?