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

Update comparison to pyregion #123

Merged
merged 3 commits into from
May 8, 2017
Merged

Conversation

joleroi
Copy link
Contributor

@joleroi joleroi commented May 5, 2017

This PR adresses #45

It adds a script to compare speed and completeness of the DS9 region parsing to pyregion. It does not check correctness. This must be done manually for each region type, I guess though.

The result confirms what we already know. The regions package is far away from providing a complete DS9 support, but if it works it's much faster than pyregion.


          filename          time_regions time_pyregion compl_pyregion compl_regions
--------------------------- ------------ ------------- -------------- -------------
        ds9.linear.wcsi.reg       -1.000         0.735          91.4%          0.0%
     ds9.mosaic.fk4.hms.reg       -1.000         0.692          91.4%          0.0%
              ds9.color.reg        0.005         0.220         100.0%        100.0%
          ds9.fk5.strip.reg       -1.000        -1.000           0.0%          0.0%
                ds9.fk5.reg       -1.000         0.443          91.7%          0.0%
     ds9.physical.strip.reg       -1.000        -1.000           0.0%          0.0%
   ds9.physical.windows.reg       -1.000         0.407          91.4%          0.0%
            ds9.fk5.hms.reg       -1.000         0.441          91.7%          0.0%
           ds9.icrs.hms.reg       -1.000         0.441          91.7%          0.0%
         ds9.icrs.strip.reg       -1.000        -1.000           0.0%          0.0%
       ds9.color.spaces.reg        0.005         0.219         100.0%        100.0%
ds9.mosaic.ecliptic.hms.reg       -1.000         0.690          91.4%          0.0%
         ds9.linear.wcs.reg       -1.000         0.680          91.4%          0.0%
    ds9.mosaic.ecliptic.reg       -1.000         0.691          91.4%          0.0%
        ds9.linear.wcsc.reg       -1.000         0.714          91.4%          0.0%
      ds9.fk4.hms.strip.reg       -1.000        -1.000           0.0%          0.0%
    ds9.mosaic.galactic.reg       -1.000         0.691          91.4%          0.0%
        ds9.linear.wcsd.reg       -1.000         0.720          91.4%          0.0%
     physical_reference.reg        0.001         0.208         100.0%        100.0%
                ds9.fk4.reg       -1.000         0.439          91.7%          0.0%
        ds9.image.strip.reg       -1.000        -1.000           0.0%          0.0%
     ds9.icrs.hms.strip.reg       -1.000        -1.000           0.0%          0.0%
              ds9.image.reg       -1.000         0.412          91.7%          0.0%
     ds9.mosaic.fk5.hms.reg       -1.000         0.691          91.4%          0.0%
       ds9.mosaic.image.reg       -1.000         0.681          91.4%          0.0%
           ds9.physical.reg       -1.000         0.413          91.7%          0.0%
           ds9.ecliptic.reg       -1.000         0.443          91.7%          0.0%
     ds9.ecliptic.strip.reg       -1.000        -1.000           0.0%          0.0%
               ds9.fits.reg       -1.000         0.242         100.0%          0.0%
               ds9.icrs.reg       -1.000         0.442          91.7%          0.0%
         ds9.mosaic.fk4.reg       -1.000         0.692          91.4%          0.0%
    ds9.mosaic.icrs.hms.reg       -1.000         0.694          91.4%          0.0%
       ds9.ecliptic.hms.reg       -1.000         0.440          91.7%          0.0%
            ds9.fk4.hms.reg       -1.000         0.441          91.7%          0.0%
ds9.mosaic.galactic.hms.reg       -1.000         0.688          91.4%          0.0%
        ds9.mosaic.icrs.reg       -1.000         0.693          91.4%          0.0%
            ds9.comment.reg       -1.000        -1.000           0.0%          0.0%
          fk5_reference.reg        0.009         0.836         100.0%        100.0%
         ds9.mosaic.fk5.reg       -1.000         2.767          91.4%          0.0%
          ds9.fk4.strip.reg       -1.000        -1.000           0.0%          0.0%
     ds9.galactic.strip.reg       -1.000        -1.000           0.0%          0.0%
           ds9.galactic.reg       -1.000         0.442          91.7%          0.0%
          ds9.composite.reg       -1.000         0.441          91.7%          0.0%
        ds9.linear.wcsa.reg       -1.000         2.373          91.4%          0.0%
        ds9.linear.wcsp.reg       -1.000         2.176          91.4%          0.0%
      ds9.fk5.hms.strip.reg       -1.000        -1.000           0.0%          0.0%
       ds9.galactic.hms.reg       -1.000         0.586          91.7%          0.0%
     galactic_reference.reg        0.002         0.827         100.0%        100.0%
 ds9.galactic.hms.strip.reg       -1.000        -1.000           0.0%          0.0%
    ds9.mosaic.physical.reg       -1.000         1.550          91.4%          0.0%
 ds9.ecliptic.hms.strip.reg       -1.000        -1.000           0.0%          0.0%

@cdeil cdeil requested a review from keflavich May 6, 2017 22:52
@cdeil cdeil self-assigned this May 6, 2017
@cdeil cdeil added this to the 0.3 milestone May 6, 2017
@cdeil
Copy link
Member

cdeil commented May 6, 2017

@joleroi - Thanks!

I think it might be nice to have the result table (with some comments above or below summarising the table) in the docs (in some non-prominent page) to show status? What is the goal here, have 100% for all the files? Is that possible? Why doesn't pyregion achieve that?

Or if you don't think the table belongs in the docs, just put the results table somewhere in the repo so that one can see it and link to it. (Github renders .cvs files nicely as tables now, but also markdown or rst work).

with open(str(filename)) as origin_file:
for line in origin_file:
if '(' in line:
n_regions += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

should be n_regions += line.count("(") since there are some example files that have ;-separated regions instead of newline-separated.

@joleroi
Copy link
Contributor Author

joleroi commented May 8, 2017

@keflavich
Thanks for the comment
@cdeil
I added the csv file to the repo for further discussion

ok to merge?

@joleroi joleroi merged commit a16641d into astropy:master May 8, 2017
@joleroi joleroi deleted the pyregion_comparison branch May 8, 2017 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants