doc/tutorial: adding a tutorial for MorphFuncy and proper docstring#215
doc/tutorial: adding a tutorial for MorphFuncy and proper docstring#215sbillinge merged 5 commits intodiffpy:mainfrom
Conversation
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
nicely done. Please see comments.
doc/source/quickstart.rst
Outdated
| ``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.
doc/source/quickstart.rst
Outdated
|
|
||
| 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.
|
|
||
| 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.
doc/source/quickstart.rst
Outdated
| 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()"
doc/source/quickstart.rst
Outdated
| 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
doc/source/quickstart.rst
Outdated
| 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.
doc/source/quickstart.rst
Outdated
| 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?
| 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
|
|
||
| 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.
|
@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. |
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 🚀 New features to boost your workflow:
|
2 participants
Hi Simon, I have added a tutorial for the
MorphFuncy. Note thatMorphFuncyhas to be used with the API. For theMorphSqueezeAndrew 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
MorphFuncyandMorphSqueezeto be consistent with the other morphs. In order to see the changes and how it looks in I think needs to be merged first?