Skip to content

Comments

Add support for specifying a separator in the image data layer#4203

Closed
elezar wants to merge 3 commits intoBVLC:masterfrom
elezar:feature/label_separator
Closed

Add support for specifying a separator in the image data layer#4203
elezar wants to merge 3 commits intoBVLC:masterfrom
elezar:feature/label_separator

Conversation

@elezar
Copy link
Contributor

@elezar elezar commented May 24, 2016

Although #3653 has been open for a while now, I noticed that #4059 (which adds support for file names with spaces in) means that my implementation may require a little more work. I therefore thought that I would break things up into multiple PRs so that I can get a better grip on the changes required.

This PR will add support for a specifying a separator (e.g. a comma) instead of assuming a space-separated list. This was also mentioned in #3433 (comment).

I have also restructured the test class somewhat to make it easier to write tests for the image data layer.

@elezar
Copy link
Contributor Author

elezar commented May 30, 2016

@flx42 @longjon since you had a look at #4218 could you have a look at this PR too?

@ajtulloch
Copy link
Contributor

This looks really clean, and the tests are nice. Great work! @flx42 @longjon @shelhamer want to take a look?

@flx42
Copy link
Contributor

flx42 commented Jun 1, 2016

I haven't tested it, but it looks clean indeed. Probably OK, Nice work!

I'm just wondering if we should squash 7638ecc and 0fe24fd.

@elezar
Copy link
Contributor Author

elezar commented Jun 1, 2016

Thanks for taking a look. The combination of the two commits probably makes
more sense. I will get it done.

@elezar elezar force-pushed the feature/label_separator branch from a87bdc2 to ffd521c Compare June 1, 2016 05:48
Evan Lezar added 3 commits June 1, 2016 07:51
- Add a separator to the proto definition.
- Add a test for a separator that is too long.
- Added a comma-separated test.
@elezar elezar force-pushed the feature/label_separator branch from ffd521c to 0925d9a Compare June 1, 2016 05:52
@elezar
Copy link
Contributor Author

elezar commented Jun 1, 2016

@flx42 I have rebased the commits (and onto the current master) as requested. I hope that everything is in order now.

@elezar
Copy link
Contributor Author

elezar commented Jul 26, 2017

Any update on this? Would be great to get this merged and closed!

@elezar
Copy link
Contributor Author

elezar commented Aug 13, 2018

Closing this PR as stale.

@elezar elezar closed this Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants