Skip to content

Fix broken import of rgb2gray in benchmark suite#4176

Merged
soupault merged 1 commit intoscikit-image:masterfrom
lagru:fix-bench-suite
Sep 22, 2019
Merged

Fix broken import of rgb2gray in benchmark suite#4176
soupault merged 1 commit intoscikit-image:masterfrom
lagru:fix-bench-suite

Conversation

@lagru
Copy link
Copy Markdown
Member

@lagru lagru commented Sep 22, 2019

Description

Running the benchmark suite seems to fail with an ImportError after in 17d6347 (#3265). I'm not sure if rgb2gray was intentionally left out or forgotten.The module docstring seems to indicate the former.

Benchmark suite passes on my local machine with asv check. Might be useful to add this to the CI somewhere. Maybe as a periodical job for the master?

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

Broken after making the import more explicit in
17d6347.
@lagru lagru added the 🤖 type: Infrastructure CI, packaging, tools and automation label Sep 22, 2019
@lagru lagru changed the title Fix broken import in benchmark suite Fix broken import of rgb2gray in benchmark suite Sep 22, 2019
@lagru
Copy link
Copy Markdown
Member Author

lagru commented Sep 22, 2019

Unrelated question: is the scope for labels written down somewhere?

@soupault soupault merged commit ef7b2e7 into scikit-image:master Sep 22, 2019
@soupault
Copy link
Copy Markdown
Member

Thanks @lagru !

is the scope for labels written down somewhere?

Could you elaborate on your question? I'm not sure which labels are you referring to.

@lagru
Copy link
Copy Markdown
Member Author

lagru commented Sep 22, 2019

Oh, I mean the ones used to label issues and PRs. E.g. for this PR I wasn't sure whether "infrastructure", "performance" or "maintenance" where the applicable. I'd be interested if the intention is written down somewhere like for NumPy's commit acronyms. I searched the mailing list and the repo but not thoroughly.

I guess newcomers can implicitly learn how labels are used overtime but a short explanation might be a nice addition to the Core Developer Guide.

@lagru lagru deleted the fix-bench-suite branch September 22, 2019 10:46
@lagru
Copy link
Copy Markdown
Member Author

lagru commented Sep 22, 2019

Just saw, GitHub supports descriptions for its labels which is an even better place than the mentioned guide.

@sciunto sciunto added this to the 0.16 milestone Sep 22, 2019
@sciunto
Copy link
Copy Markdown
Member

sciunto commented Sep 22, 2019

@lagru we've never done this.

@soupault
Copy link
Copy Markdown
Member

👍 to @sciunto . I introduced labels to skimage repo few years ago to help us better triage the issues and PRs. However, I don't think we have a clear definition of them still as their benefit remains limited.
The most structured discussion on this topic is happening in #3629.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 type: Infrastructure CI, packaging, tools and automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants