Include 2D data treatment #10
Conversation
|
This "This branch has conflicts that must be resolved" is a new situation for me, but I will follow (quasi-blindly) the suggestions github sent me. |
|
Trying closing and making the pull request again |
Codecov Report
@@ Coverage Diff @@
## master #10 +/- ##
===========================================
+ Coverage 43.26% 59.13% +15.87%
===========================================
Files 3 4 +1
Lines 282 394 +112
===========================================
+ Hits 122 233 +111
- Misses 160 161 +1
Continue to review full report at Codecov.
|
|
Rebased, because that's what we do for fun on a Saturday night at a SciPy conference. Rebasing across a reversion commit is a pretty rough first encounter with "This branch has conflicts that must be resolved", so I'm happy to take this one. |
|
I tacked on a commit undoing the |
|
I asked for the @pytest fixture based on a review by @danielballan of other code I have done. If he thinks it isn't necesary here I am happy with that. (@danielballan perhaps at some point we can go over when/where to use pytest fixtures because it appears I am still not fully understanding that. |
rixs/process2d.py
Outdated
| @@ -0,0 +1,240 @@ | |||
| """ Functions for processing 2D Image data. | |||
|
|
|||
| Typical command line workflow is executed in run_rest() | |||
There was a problem hiding this comment.
I think we had discussed removing 'run_test()' so this should be updated with an actual typical comand line workflow.
There was a problem hiding this comment.
I guess there are a few different ways to guide the user here regarding how the package should be used. I think it would be better to delete this line and include this in the sphynx documentation.
There was a problem hiding this comment.
Agreed, this is a better option for a detailed discussion of how to run the software
| def estimate_elastic_pos(photon_events, x_range=(0, 20), bins=None): | ||
| """ | ||
| Estimate where the elastic line is. | ||
|
|
There was a problem hiding this comment.
I think we should remove one of these empty lines
|
|
||
| def make_fake_image(curvature, elastic_y, sigma=2, noise=0.002): | ||
| """Make a fake list of photon events. | ||
| Parameters |
There was a problem hiding this comment.
I think we are missing a blank line here
rixs/process2d.py
Outdated
| return elastic_y_value | ||
|
|
||
|
|
||
| def extract(photon_events, curvature, bins=None): |
There was a problem hiding this comment.
I am still not very keen on this function being called 'extract' as I think it makes reading code using this confusing.
Is there a reason this can't be renamed to something that more accurately describes it's function, a suggestion (but something else that is clearer is also welcomed) is 'apply_curvature'... This matches well with the other names used here (get_curvature_offset, fit_curvature, ....).
There was a problem hiding this comment.
Changed to apply_curvature
rixs/process2d.py
Outdated
| curvature : array | ||
| The polynominal coeffcients describing the image curvature. | ||
| These are in decreasing order e.g. | ||
| curvature[0]*x^2 + curvature[1]*x^1, + curvature[2]*x^0, |
There was a problem hiding this comment.
I think placing this inside a code-block will be a good idea: this can be done using the syntax:
.. code-block:: python
curvature[0]*x^2 + curvature[1]*x^1 + curvature[2]*x^0
Alternatively it could be done inside as math, with the same syntax as above but with 'code-block:: python' replaced with 'math::'
There was a problem hiding this comment.
Will do. Since this line is now code rather than just an illustration. I will change ^ to **
.. code-block:: python
curvature[0]*x**2 + curvature[1]*x**1 + curvature[2]*x**0
|
Looks good to me. |
2D RIXS data treatment routines.
Plotting functions are now in a different module.
if name == “main” test removed
Slightly changed in order to use numpy.fitpoly. The lmfit dependency has been removed.
Example notebooks are saved without them having been run.