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

[MRG+1] Insert metadata in docstrings of images in skimage.data.* #2236

Merged
merged 1 commit into from
Sep 3, 2016

Conversation

sciunto
Copy link
Member

@sciunto sciunto commented Aug 5, 2016

This PR solves the first item of #989 with the recommendation of @stefanv #993 (comment)

I used a similar function name as in regionprops.

This screenshot illustrates the result. I'm open to comments, especially on the style we want...

metadata

Cc @cdeil

@soupault soupault added 📄 type: Documentation Updates, fixes and additions to documentation ⏩ type: Enhancement Improve existing features labels Aug 5, 2016
@codecov-io
Copy link

codecov-io commented Aug 5, 2016

Current coverage is 90.66% (diff: 100%)

Merging #2236 into master will increase coverage by 0.06%

@@             master      #2236   diff @@
==========================================
  Files           304        303     -1   
  Lines         21435      21773   +338   
  Methods           0          0          
  Messages          0          0          
  Branches       1844       2014   +170   
==========================================
+ Hits          19421      19741   +320   
- Misses         1661       1669     +8   
- Partials        353        363    +10   

Powered by Codecov. Last update d984b38...3434623

def _install_metadata_doc():
"Insert metadata to docstrings that return images."
import sys
for name in __all__:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think I'm a fan of this approach. What if we want to add new functions? It's up to the contributor to figure out that this global function is messing with their contribution.

Instead, I would suggest a decorator that gets applied to each image function. It's a bit more wordy, but also more self-documenting.

As an alternative, I would make this list explicit, instead of implicitly loading all names.

@@ -259,3 +259,20 @@ def rocket():

"""
return load("rocket.jpg")

def _install_metadata_doc():
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can do this, since it requires loading all images upon import.

Can we just hardcode the meta-data?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok. Do you have a preferred format? (Just to make it right with a single bullet :) )

@sciunto
Copy link
Member Author

sciunto commented Sep 3, 2016

function to print metadata.

def f():
    from skimage import data
    import sys
    for name in data.__all__:
        if name in ('load', 'lena'):
            continue
        func = getattr(sys.modules[data.__name__], name)
        metadata = "    Shape: %s; dtype: %s" % (func().shape, func().dtype)
        print(name)
        print(metadata)

@sciunto
Copy link
Member Author

sciunto commented Sep 3, 2016

I inserted manually the metadata. I posted a code to double check.

@soupault
Copy link
Member

soupault commented Sep 3, 2016

Looks good.

@soupault soupault changed the title Insert metadata in docstrings of images in skimage.data.* [MRG+1] Insert metadata in docstrings of images in skimage.data.* Sep 3, 2016
@sciunto
Copy link
Member Author

sciunto commented Sep 3, 2016

Looks like the random travis failures are due to #2237

@emmanuelle
Copy link
Member

Travis is now happy: merging!

@emmanuelle emmanuelle merged commit 3187cb4 into scikit-image:master Sep 3, 2016
@sciunto sciunto deleted the data_metadata branch September 3, 2016 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features 📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants