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

Enhancement wishlist #47

Open
ljwolf opened this issue Dec 20, 2019 · 3 comments
Open

Enhancement wishlist #47

ljwolf opened this issue Dec 20, 2019 · 3 comments

Comments

@ljwolf
Copy link
Member

ljwolf commented Dec 20, 2019

This is a running list of enhancements burbling up of when writing the gdsbook/book chapter on point pattern analysis.

additional hulling measures

  • hull() should be general, with a type argument that supports alpha shapes, minimum bounding rectangles, minimum area rectangles, and minimum bounding circles. This would require putting a dedicated convex_hull function elsewhere. Since we could set hull's default to be the convex hull, this can actually be made API-transparent 😄
  • mbr should be expanded to minimum_bounding_rectangle
  • we should make an import-safe minimum_area_rectangle and borrow/import the opencv implementation of the minimum area rectangle.
  • we should rename skyum to minimum_bounding_circle, or use it as a fallback if we cannot use opencv's minimum bounding circle
  • docstring for skyum needs to be brought in line with other functions/classes (my bad 😥)

API consistency

  • Elsewhere in the library, we avoid exposing everything at root now. Here, though, everything in centrography is available directly in pointpats... is this intentional?
  • skyum should return in the same manner as ellipse. I think they both should give some kind of namedtuple return value that has (center, radius) or (center, semimajor, semiminor, rotation).

Performance

  • mbr does a loop in python through all points and explicitly finds the minimum. We should probably use a sort for a faster solution?
  • skyum can be easily numba-ized, but I'm not sure if it'd matter much.

New statistics

  • local K/F functions
@ljwolf ljwolf changed the title Myriad enhancements Enhancement wishlist Dec 20, 2019
@sugam45
Copy link
Contributor

sugam45 commented Mar 16, 2020

Hi @ljwolf
I have analyzed the points put forth by you.
In my analysis :

  • A generic hull function is a really good suggestion. This can be efficiently done via a generic hull class importing all other functions.

  • I agree with the import safe suggestion. This will really help in removing breakpoints in our code.

  • Sort for mbr may not be a prudent choice (sort being a O(N log N) ). Instead, we can use a comparison in pairs strategy.

  • We can have a namedtuple each place functions are getting referenced.

  • I think that numba-ization can be done for skyum. We can do it via parallel threading

@ljwolf
Copy link
Member Author

ljwolf commented Mar 17, 2020

Hey thanks!
If you want to take a stab at it,

  1. wrapping the opencv minimum area rectangle into a special function
  2. renaming mbr to minimum bounding rectangle

would be a good first steps for this!

@sugam45
Copy link
Contributor

sugam45 commented Mar 17, 2020

Hi @ljwolf
I have already made some changes in the code for mbr loop.

  • Instead of a loop, the tuple array has been divided into 2 arrays and separate max and min have been calculated.

  • Also, I have renamed mbr to minimum_bounding_rectangle.

  • Currently, I am working on the wrapper for opencv minimum area rectangle.

I will raise the PR for the same after proper testing.

sugam45 added a commit to sugam45/pointpats that referenced this issue Mar 17, 2020
Changed mbr to minimum_bounding_rectangle,
Added opencv minimum_area_rectangle,
Removed loop in mbr and substituted with in-built max and min functions
sugam45 added a commit to sugam45/pointpats that referenced this issue Mar 17, 2020
Added opencv to requirements and conf.py
sugam45 added a commit to sugam45/pointpats that referenced this issue Mar 17, 2020
Removed minz from sys.float_info
sugam45 added a commit to sugam45/pointpats that referenced this issue Mar 17, 2020
Resolved precision error
sjsrey added a commit that referenced this issue Jul 4, 2020
#47 : Added enhancements to mbr in pointpats
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants