Closed barucden closed 2 years ago
To not break your dev workflow, I made a PR to https://github.com/barucden/Augmentor.jl/pull/1, you can review and merge if that looks good to you.
Ideally, we could do this in src/codegen.jl
, but that's more difficult to handle. I'm not an expert in metaprogramming neither.
I had to explicitly define the type of img
for all augment
methods to remove ambiguities. I hope it's fine.
Looking good to me!
Fighting with method ambiguity isn't a very interesting journey, do you think we need to introduce some new function e.g., semantic_augment
for this and all future extensions? This might make it more clear that augment
is more faithful, and semantic_augment
as an extension to augment
allows more interpretations. The "semantic" in semantic_augment
is not preserved for semantic segmentation, of course.
do you think we need to introduce some new function e.g., semantic_augment for this and all future extensions?
I am not sure. I can imagine that the ambiguity is an issue if more conventions (like the pair notation) are introduced. But do you think there will be more? As of now, I think it is manageable.
There was also an issue with Either
. This "meta" operation needs a special treatment.
done. welcome!
@barucden merge when you're ready.
@Evizero thank you for the invitation!
@johnnychen94 I am thinking I should add one more test to check all the remaining operations cooperate with semantic wrappers. Then I think it is safe to merge.
One last API thing: I think it is worthwhile to implement an augment version for named tuples, while you're at it. That would be super clear for users and I think also extensible. But either way, I'm already super happy with the current implementation - thanks for the effort!
Since we only support masks now, would this API bring something "new"? Seems like a more verbose version to the pair version:
augment(img => mask, pl) # can do now
augment((image=img, mask=mask), pl) # named tuple version
I can see it being useful once we support more targets (besides masks), so maybe we should decide the API when we implement additional targets?
Agreed. Adding support for NamedTuple can be considered when there is a new application that this PR doesn't cover. By then it can be easier to provide a justified API.
Thanks again mate! It‘s just a small PR, but a big step for julia adoption in my work :)
Glad to hear! Give it a try. It certainly needs some "battle testing" :)
@barucden in most cases, we follow the continuous release strategy. So I've made a patch release https://github.com/Evizero/Augmentor.jl/commit/ad9a3984921b51c36bb73a75fdba3097a70041f9
https://www.oxinabox.net/2019/09/28/Continuous-Delivery-For-Julia-Packages.html
Oh, okay, got it! Thanks
This PR introduces support for segmentation masks. The key principle is that we do not apply some operations on segmentation masks.
That being said, the implementation is not finished yet, and I would like to kindly ask for assistance. I am not sure how to implement the "operation skipping", or perhaps where to implement it. I am thinking maybe operation.jl#L57 and operation.jl#L60 so it would result in
But I am not convinced.
TODO:
SemanticWrapper
interfaceCloses #85