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

Img proc math #62

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Img proc math #62

wants to merge 34 commits into from

Conversation

giltis
Copy link
Contributor

@giltis giltis commented Jun 21, 2015

Supersedes #57 and #54

Gabriel Iltis and others added 30 commits April 8, 2015 14:24
After review of the arithmetic tools with Eric, we decided that tools
whos sole purpose is to wrap existing tools into more user friendly
VisTrails modules should be located in VTTools, thereby leaving skxray
to exclusively contain new analysis tools and functions, while
continuing to increase the usefullness and usability of VisTrails for
analysis of NSLS-II data.

The file mathops.py has been moved to vttools/to_wrap and renamed as
image_proc.py. The file test_img_proc.py has been copied from
skxray/tests/ to vttools/tests/.
…ails

As per review and comment from Eric, the arithmetic functions designed
explicitly to ease image processing and data analysis in VisTrails have
been moved to the VTTools repo. This commit removes the addl analysis
funcs, which will remain in skxray (logical_nand, logical_nor,
logical_sub). The remaining functions (arithmetic_basic,
arithmetic_custom, and logic_basic) remain.

All additional math functions are directly imported from either mathops
or numpy. And names for mathops functions have been corrected to mimic
style established in Numpy.
Simple term replacement. Replaced all references to mathops by changing
import statement to:
import vttools.to_wrap.image_proc as img
The tests included focus on the vistrails specific functions:
arithmetic_basic, logic_basic, and arithmetic_custom
Previously the docstring for arithmetic_custom included references
to additional operators such as +=, -=, *= etc. which are supposed to
simplify expressions like a=a+b, which is the equivalent of a+=b.
However, the parsing function does not evaluate these short cuts
correctly.

If these additional operators become important, then I'll revisit and
modify the parsing function, or write a custom one. However, at this
time the additional operators are deemed to be unnecessary, since the
expressions can still be evaluated without the operators.`
As per Eric's suggestion, inputs specific to data have been renamed in
order to conform to typical numpy docstrings, e.g. the arg src_data1
and src_data2 have been changed to x1 and x2.

The funcs requiring a string operation to be defined have also been
standardized so that the input order starts with the operation
or expression followed by the input data.

Finally, the docstring for arithmetic_custom was incomplete, and has
been flushed out.
As discussed with Eric, we decided to shorten, simplify and make the
function names more explicit about their functionality. So, logic_basic
has been renamed to logic, arithmetic_basic has been renamed arithmetic,
and arithmetic_custom has been changed to
arithmetic_expression. In Vistrails these tools are exprected
to be nested in the tool tree under an "Image Arithmetic" heading,
included in the "Image Processing" tool set/kit.
After review of the arithmetic tools with Eric, we decided that tools
whos sole purpose is to wrap existing tools into more user friendly
VisTrails modules should be located in VTTools, thereby leaving skxray
to exclusively contain new analysis tools and functions, while
continuing to increase the usefullness and usability of VisTrails for
analysis of NSLS-II data.

The file mathops.py has been moved to vttools/to_wrap and renamed as
image_proc.py. The file test_img_proc.py has been copied from
skxray/tests/ to vttools/tests/.
…ails

As per review and comment from Eric, the arithmetic functions designed
explicitly to ease image processing and data analysis in VisTrails have
been moved to the VTTools repo. This commit removes the addl analysis
funcs, which will remain in skxray (logical_nand, logical_nor,
logical_sub). The remaining functions (arithmetic_basic,
arithmetic_custom, and logic_basic) remain.

All additional math functions are directly imported from either mathops
or numpy. And names for mathops functions have been corrected to mimic
style established in Numpy.
Simple term replacement. Replaced all references to mathops by changing
import statement to:
import vttools.to_wrap.image_proc as img
The tests included focus on the vistrails specific functions:
arithmetic_basic, logic_basic, and arithmetic_custom
Previously the docstring for arithmetic_custom included references
to additional operators such as +=, -=, *= etc. which are supposed to
simplify expressions like a=a+b, which is the equivalent of a+=b.
However, the parsing function does not evaluate these short cuts
correctly.

If these additional operators become important, then I'll revisit and
modify the parsing function, or write a custom one. However, at this
time the additional operators are deemed to be unnecessary, since the
expressions can still be evaluated without the operators.`
As per Eric's suggestion, inputs specific to data have been renamed in
order to conform to typical numpy docstrings, e.g. the arg src_data1
and src_data2 have been changed to x1 and x2.

The funcs requiring a string operation to be defined have also been
standardized so that the input order starts with the operation
or expression followed by the input data.

Finally, the docstring for arithmetic_custom was incomplete, and has
been flushed out.
As discussed with Eric, we decided to shorten, simplify and make the
function names more explicit about their functionality. So, logic_basic
has been renamed to logic, arithmetic_basic has been renamed arithmetic,
and arithmetic_custom has been changed to
arithmetic_expression. In Vistrails these tools are exprected
to be nested in the tool tree under an "Image Arithmetic" heading,
included in the "Image Processing" tool set/kit.
- This looks like a lot of changes, but there is functionally
  no difference between the original code and this code.  It is
  entirely stylistic.
- Doc edits moved the descriptive prose to `/doc/resource/user-guide/image.rst`
  and used language largely copied from numpy
- content edits favored use of built in functionality over
  defining look-up dictionaries
Conflicts:
	vttools/tests/test_img_proc.py
	vttools/to_wrap/image_proc.py
The tools designed to ease use of math functions in VisTrails are now
included in vttools-->to_wrap-->image_processing-->arithmetic since
these tools are explicitly designed to simplify the VisTrails user
experience. As part of the growing collection of image processing
functions included in skxray, the basic image arithmetic functions
are compiled in skxray-->image_processing-->arithmetic for
completeness.
@ericdill
Copy link
Contributor

You seem to have added an html/ folder to this branch in bdbe90d. Can you remove that?

See Also
--------
- User guide section on "Image Operations"
('/doc/resource/user-guide/image.rst')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a todo here noting that this needs to be updated when there are sphinx docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought you added that. Which is why I left it in....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs updated with the TODO statement.

Gabriel Iltis added 3 commits June 22, 2015 14:16
HTML folder was accidentally added during a previous commit. This
folder has now been removed in order to merge this PR finally.
As Eric mentioned, lmfit.asteval may be a cleaner and more effective
parsing tool for evaluating the input expressions. Added an explicit
TODO noting the parser comparision should be completed on next
iteration.
@giltis
Copy link
Contributor Author

giltis commented Jun 23, 2015

I'm assuming that Travis is failing because the required pkg hasn't been merged into skxray? Just want to make sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants