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

Added kernel_2d classes to numeric extension #361

Merged
merged 1 commit into from Aug 4, 2019

Conversation

miralshah365
Copy link
Contributor

@miralshah365 miralshah365 commented Jul 31, 2019

Description

for more detail check the reference(s)

References

#356

Tasklist

@miralshah365 miralshah365 self-assigned this Jul 31, 2019
@miralshah365 miralshah365 force-pushed the kernel_2d branch 2 times, most recently from 5783bbb to 39dee36 Compare Jul 31, 2019
@miralshah365 miralshah365 requested a review from mloskot Jul 31, 2019
@mloskot mloskot added the cat/feature label Jul 31, 2019
@mloskot mloskot marked this pull request as ready for review Jul 31, 2019
Copy link
Member

@mloskot mloskot left a comment

After the first round of the review, I have three issues:

  1. The storage is not optimal (see #361 (comment))
  2. The indexing is ambiguous (see #361 (comment))
  3. The PR subject 2D kernel support added suggests algorithms operating on 2D kernels have been also included, what does not seem to be the case. I'd suggest to change it to something like "Add 2D kernel definitions to numeric extension"

include/boost/gil/extension/numeric/kernel.hpp Outdated Show resolved Hide resolved
include/boost/gil/extension/numeric/kernel.hpp Outdated Show resolved Hide resolved
test/extension/numeric/kernel.cpp Show resolved Hide resolved
@mloskot mloskot added the status/work-in-progress label Jul 31, 2019
@miralshah365 miralshah365 changed the title 2D kernel support added to numeric extension Added 2D kernel definitions to numeric extension Aug 1, 2019
@miralshah365 miralshah365 force-pushed the kernel_2d branch 3 times, most recently from 020d49c to 49dc620 Compare Aug 3, 2019
Copy link
Member

@mloskot mloskot left a comment

@miralshah365 I added one extra request to replace assertions with exceptions. Other than that, it looks good to me. Once you make this change and CI-s pass, feel free to merge

include/boost/gil/extension/numeric/kernel.hpp Outdated Show resolved Hide resolved
@miralshah365 miralshah365 force-pushed the kernel_2d branch 2 times, most recently from 56ff47b to a7913bc Compare Aug 3, 2019
Tests for 2D numeric kernel added
@miralshah365 miralshah365 merged commit 11c897b into boostorg:develop Aug 4, 2019
8 of 9 checks passed
[gsoc2019] Image Processing Algorithms automation moved this from In progress to Done Aug 4, 2019
@mloskot mloskot self-requested a review Aug 5, 2019
@mloskot mloskot removed the status/work-in-progress label Aug 5, 2019
Copy link
Member

@mloskot mloskot left a comment

Since the request in #361 (review) has been addressed, I'm happy to approve this.

@mloskot mloskot changed the title Added 2D kernel definitions to numeric extension Added kernel_2d to numeric extension Oct 24, 2019
@mloskot mloskot changed the title Added kernel_2d to numeric extension Added kernel_2d classes to numeric extension Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/feature
Development

Successfully merging this pull request may close these issues.

None yet

2 participants