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 #289

Merged
merged 3 commits into from
Jun 23, 2015
Merged

Img proc math #289

merged 3 commits into from
Jun 23, 2015

Conversation

giltis
Copy link

@giltis giltis commented Jun 21, 2015

Minor docstring correction and organizational refactor. Finally changing module name from mathops to more clearly understandable name of arithmetic

Gabriel Iltis added 2 commits June 21, 2015 06:04
Input types were assigned as either array-like or array_like.
Any input type that was specified as array_like has now been changed to
array-like for consistency.
In an effort to simplify the user experience for image processing,
image processing folders and modules are being refactored to more
clearly state what kind of tools each module contains. This
initial refactor specifically changes the folder name:
skxray/img_proc to skxray/image_processing and the module mathops.py to
arithmetic.py. This also addresses a long standing question from Tom
about why I was beholden to the name mathops (which stood for math
operations). I'm hoping that these changes add an additional level of
clarity.
@tacaswell
Copy link
Member

This needs a note in https://github.com/Nikea/scikit-xray/tree/master/doc/api_changes documenting the API change.

@danielballan
Copy link
Member

Probably have to update setup.py as well.

On Sun, Jun 21, 2015 at 7:04 AM Gabriel Iltis notifications@github.com
wrote:

Minor docstring correction and organizational refactor. Finally changing

module name from mathops to more clearly understandable name of arithmetic

You can view, comment on, or merge this pull request online at:

#289
Commit Summary

  • DOC: Removed inconsistencies in spelling of array-like
  • DEV: Refactor-img_proc to image_processing, mathops to arithmetic

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#289.

@ericdill
Copy link
Member

I would also advocate for turning image.py into a package and putting arithmetic in that package instead of image_processing

Documented the renaming of folder img_proc to image_processing, and
renaming of module mathops.py to arithmetic.py. These changes were
made in order to help clarify function and tool groups for users.
@giltis
Copy link
Author

giltis commented Jun 23, 2015

@ericdill : So you're thinking of simply including a package called 'image' instead of 'image processing' ?

@giltis
Copy link
Author

giltis commented Jun 23, 2015

@danielballan : There is a very real possibility that I'm missing something here, but I'm not seeing where, or why a change to setup.py should be made.

@sameera2004
Copy link
Member

But there is a module called 'image.py' already in skxray? It has some image processing tools too, (@tacaswell finding the center of the ring). May be, you can integrate everything into image package?

@ericdill
Copy link
Member

All of the modules in skxray/ (feature.py, image.py, recip.py, etc...) will eventually turn in to packages. They are not packages right now because they are not sufficiently large to warrant their own packages. I'd suggest the following rearrangements to image.py and image_processing/arithmetic.py:

skxray/
   image/
      __init__.py (import find_ring_center_acorr_1D here, there will be no api break this way)
      ring.py (move the code from `image.py` here)
      arithmetic.py

@ericdill
Copy link
Member

re: setup.py. As the setup.py file of skxray is currently written, there is no need to change it (https://github.com/Nikea/scikit-xray/blob/master/setup.py#L21). What @danielballan is likely referring to is how setup.py is done in many of our other libraries (see dataportal, for example https://github.com/NSLS-II/dataportal/blob/master/setup.py#L65) where we are explicitly listing each and every subpackage in the library. Because you added a new subpackage image_processing you would need to update the setup.py file if we were listing all subpackages explicitly.

tacaswell added a commit that referenced this pull request Jun 23, 2015
API: re-arrange some of the image processing tools
@tacaswell tacaswell merged commit 7a32bab into scikit-beam:master Jun 23, 2015
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.

5 participants