-
Notifications
You must be signed in to change notification settings - Fork 19
Add RandomAccessiblePair randomAccess test #31
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
Conversation
hanslovsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A list of small modifications that improve readability.
| Img< ByteType > in = new ArrayImgFactory< ByteType >().create( new FinalDimensions( 3, 3 ), new ByteType() ); | ||
| OutOfBoundsFactory< ByteType, RandomAccessibleInterval< ByteType > > outOfBoundsFactory = | ||
| new OutOfBoundsBorderFactory<>(); | ||
| RandomAccessibleInterval< ByteType > extendedIn = Views.interval( Views.extend( in, outOfBoundsFactory ), in ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Views.extendBorder instead of creating a separate factory.
| RandomAccessibleInterval< ByteType > extendedIn = Views.interval( Views.extend( in, outOfBoundsFactory ), in ); | ||
|
|
||
| // Create RandomAccessiblePair | ||
| final RandomAccessible< Neighborhood< ByteType > > safe = new RectangleShape( 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1, should be in a new line like this (with appropriate indentation):
... = new RectangleShape(
1,
false )There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hanslovsky I have added an additional comment in the opening line so that the formatter doesn't try to move the 1 to the first line every time it's run. Let me know if I should remove it or keep it that way...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting. Are you using eclipse? I have noticed that the eclipse formatter is not always consistent among different machines (probably dependent on what settings are being used). I think the comment is fine here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was using Eclipse Neon on macOS with a weird configuration (that I don't have a clue where I got it from) instead of the imglib2-eclipse-mars-clean-up-formatter-style.xml. Sorry for the confusion!
| RandomAccessible< Pair< Neighborhood< ByteType >, ByteType > > pair = Views.pair( safe, extendedIn ); | ||
|
|
||
| // Set position out of bounds | ||
| RandomAccess< Pair< Neighborhood< ByteType >, ByteType > > randomAccess = pair |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for line break here (there are longer lines in this file).
4350648 to
8ac641e
Compare
|
Thanks for looking at this @hanslovsky! I have added a comment to one of yours for your review. |
Adds a test that fails prior to #30.