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

Work around vacuous type parameters in LabelingIOService.load #4

Open
gselzer opened this issue Apr 6, 2022 · 6 comments
Open

Work around vacuous type parameters in LabelingIOService.load #4

gselzer opened this issue Apr 6, 2022 · 6 comments

Comments

@gselzer
Copy link
Contributor

gselzer commented Apr 6, 2022

The type parameters in LabelingIOService.load() are vacuous, which could lead to some nasty ClassCastExceptions.

See #3 (comment)

@tomburke-rse
Copy link
Collaborator

So, I have been thinking about the type params.
I - the pixel value. can't really do much about that one, but at least it's an IntType
T - the label value. this is the hard one that I don't really have a feel what to do with. I could enforce Integer/IntType here because I don't see much value in anything else being used here.
S - I can potentially drop and replace with a Map<String,Object> (which is also the basic JSOn parse result) and remove the param altogether.

What are your thoughts on that?
Especially the dropping of S, but also the limiting of T would simplify the API (and codebase/complexity) by a lot.

@gselzer
Copy link
Contributor Author

gselzer commented Apr 6, 2022

My main concern is with the vacuous type parameters.

I don't think that S, the metadata type, is ever vacuous in public API. I don't think you have to make any changes to that. (There is the question of whether you need that type variable, but I don't think I know how to answer that.)

As far as the other two are concerned, I think that they must be rewritten to the broadest return allowed by ImgLabeling and/or LabelingData. LabelingData is going to hurt here because of those type parameters. Then the signature should look something like

    ImgLabeling<?, ? extends IntegerType<?>> loadNew(String file) throws IOException;

If you want nice type safety, you'd need to probably:

  • Provide Classes to identify the type variables as arguments to the method
  • Use TypeTokens

@tomburke-rse
Copy link
Collaborator

I suppose I have to go a hybrid approach or only use class arguments.
For saving, TypeTokens are manageable, as I can get the classes from the given data.
For loading, that's not going to work, but also something the implementation side might not know, but maybe I should not think so far ahead here.

I will try the hybrid approach tomorrow.

@tomburke-rse
Copy link
Collaborator

Update for today:
I'm playing around with a TypeToken as an argument to each method (or maybe a setter/adder for that)
The call would look like this then: load(String path, TypeToken<LabelingData<T,S>>)
Example:
load("Somepath", new TypeToken<LabelingData<Example,Example>>())
This looks promising so far.

The other option would be to allow the user to add (de-)serializers and call the Gson Parser API, which would result in code similar to the Bson Codecs from before.

@gselzer
Copy link
Contributor Author

gselzer commented Apr 7, 2022

load("Somepath", new TypeToken<LabelingData<Example,Example>>())

This is a step forward, for sure. Thanks @tomburke-rse

It is unfortunate, though, that it forces an explicit Gson dependency on anyone wanting to use this method.

@tomburke-rse
Copy link
Collaborator

tomburke-rse commented Apr 7, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants