Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Giuseppe Barbieri
    @elect86
    I imagine scijava just loads all the @ScijavaPlugins at the start and that's it
    Curtis Rueden
    @ctrueden
    And I'd say... maybe 20-50 types of things?
    For sure, for SciJava 2, loading everything with @Plugin would be straightforward.
    Giuseppe Barbieri
    @elect86
    also, there are some news starting from jdk16+..
    Gabriel Selzer
    @gselzer
    Alright, @ctrueden, I've been thinking more about how the TherapiOpInfoDiscoverer
    47 replies
    Curtis Rueden
    @ctrueden

    @gselzer Made some progress on the PR chain:

    I also rebased a couple of old topic branches and pushed them to stale/ prefix to clarify that they are old and may be obsolete now. I didn't finish going through all topic branches on the incubator yet, but the plan is to do that.

    Gabriel Selzer
    @gselzer
    Fantastic, many thanks @ctrueden. Did any of the merged PRs need fixes?
    2 replies
    Gabriel Selzer
    @gselzer

    @ctrueden thinking about giving the Discoverer a type variable:

    public interface Discoverer<T> {
  <U extends T> List<U> discover(Class<U> c);
}

    Any opinions?

    Curtis Rueden
    @ctrueden
    @gselzer How does it help?
    You'll end up with Discoverer<?> e.g. from the ServiceLoader.
    Gabriel Selzer
    @gselzer
    Yeah, that's kind of what I'm running up against...
    I think it would make writing Discoverer implementations much cleaner
    But in practice it would be horrible to use...
    Curtis Rueden
    @ctrueden
    It would just avoid the need for a line like:
    if (!OpInfo.class.isAssignableFrom(c)) throw new IllegalArgumentException("Cannot discover classes of type " + c.getName());
    Gabriel Selzer
    @gselzer
    Well, I think we'd want a silent failure, but yes
    Curtis Rueden
    @ctrueden
    OK, so return empty list. Sure.
    But either way you need to do the check, right?
    I think it's cleaner not to have the generic. You can ask any discoverer for any type, but some types they just don't care about.
    In which situations would having the type parameter make implementing the Discoverer easier?
    Gabriel Selzer
    @gselzer
    It would be rather nice for implementations of the TherapiDiscoverer implementations
    But I might just have to spend more time cleaning that up...

    To summarize, TherapiDiscoverer is an abstract class, whose subclasses need to implement (among less important things):

    protected abstract <U> U convert(Discovery<AnnotatedElement> e, Class<U> c);

    This method is called by 

        @Override
    public <U> List<U> discover(Class<U> c) {
        if (!canDiscover(c))
            return Collections.emptyList();
        List<Discovery<AnnotatedElement>> elements = elementsTaggedWith(tagType());
        return convertEach(elements, c);
    }
    (I've found great use for Discovery in this one instance, that class will probably be moved into SciJava Discovery Therapi
    Curtis Rueden
    @ctrueden
    Why not put the <T> on the TherapiDiscoverer base class, then?
    Gabriel Selzer
    @gselzer
    I don't think that would help there. We want to say that the U is a T
    I'm just going to leave it the way it is
    Curtis Rueden
    @ctrueden
    Sounds good.
    Giuseppe Barbieri
    @elect86
    today I saw this and immediately though at when you, Curtis, told me that the community is kind of hating Java. If you got something light to do, you can hear it in background
    Gabriel Selzer
    @gselzer
    @ctrueden so I think that I am nearly to a state that builds in the discoverer refactoring
    The remaining question is how we should give Ops/OpCollections to the StaticDiscoverer in tests
    We currently give them classes, but the Discoverers are supposed to give a list of instances
    Gabriel Selzer
    @gselzer
    @ctrueden my goal today is to try and purge SciJava Common, finally
    Starting with SciJava Types, we have this NilConverter
    I think we either:
    1. Delete the NilConverter
    2. Introduce SciJava Converter2 to the incubator
    Any opinions?
    (Additionally, Types depends on org.scijava.util.MiscUtils.compare, but I'm just going to figure out how to get the same functionality without that dependency)
    Curtis Rueden
    @ctrueden
    Get rid of the nil converter, I think. Ops doesn't need it, right?
    Gabriel Selzer
    @gselzer
    No
    Curtis Rueden
    @ctrueden
    We can put that converter in a different module downstream, if/when the need arises.
    Gabriel Selzer
    @gselzer
    :+1:
    Gabriel Selzer
    @gselzer
    Hmm, there's also a bunch of uses of FileUtils in TypesTest.
    Gabriel Selzer
    @gselzer
    @ctrueden created SciJava Common3
    Currently it only has Validated, ValidityProblem, and ValidityException in the org.scijava.common3.validity package
    Debating adding Priority to that as well
    Any opinions on Priority?
    5 replies
    Curtis Rueden
    @ctrueden
    @gselzer I'm curious if you ever read the dev.java/learn tutorials, e.g. decoupling modules with services. I'm going to check them out this week.
    5 replies
    Gabriel Selzer
    @gselzer
    @ctrueden any opinions on where VersionUtils lives in Scijava 3? For now I'm going to put it in SJC3
    Although it uses POM and Manifest (and by extension Versioned), which live in the package org.scijava.util...
    5 replies
    Gabriel Selzer
    @gselzer
    Also, for now I am going to make a scijava-priority module. I don't think it needs to be scijava-priority2, because there is no org.scijava.priority package. Happy to discuss this choice if you want
    1 reply
    Gabriel Selzer
    @gselzer

    @ctrueden my goal today is to try and purge SciJava Common, finally

    To this end, I am also going to delete PluginBasedClassOpInfoGenerator :wave:

    1 reply