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

Adding ability to load individuals from in-memory data as well as off-disk. #71

Merged
merged 5 commits into from
Mar 20, 2017

Conversation

julesjacobsen
Copy link
Contributor

Fix for @drseb in issue #57 enabling individuals data to be loaded into the knowledgebase from a Map rather than off disk.

@julesjacobsen
Copy link
Contributor Author

One question on naming. Should .loadDataFromMap(data) really be loadIndividuals? We still have no way of importing labels and presumably this requires different OWLAPI calls.

@cmungall
Copy link
Member

cmungall commented Mar 16, 2017 via email

@julesjacobsen
Copy link
Contributor Author

OK so these methods need to change their signatures too. Here 'data' means 'individualAssociations'. Does this make sense for loadDataFromOntology be changed to loadIndividualAssociationsFromOntology given these are actually just being merged? Should there just be a single set of loadOntology or loadInstancedOntology?

         public Loader loadDataFromOntology(String path) {
            sourceDataBuilder.dataOntology(path);
            return this;
        }

        public Loader loadDataFromOntologies(String... paths) {
            sourceDataBuilder.dataOntologies(paths);
            return this;
        }

        public Loader loadDataFromOntologies(Collection<String> paths) {
            sourceDataBuilder.dataOntologies(paths);
            return this;
        }

        public Loader loadDataFromTsv(String path) {
            sourceDataBuilder.dataTsv(path);
            return this;
        }

        public Loader loadDataFromTsv(String... paths) {
            sourceDataBuilder.dataTsv(paths);
            return this;
        }

        public Loader loadDataFromTsv(Collection<String> paths) {
            sourceDataBuilder.dataTsv(paths);
            return this;
        }

        public Loader loadDataFromMap(Map<String, ? extends Collection<String>> data) {
            sourceDataBuilder.data(data);
            return this;
       }

…ciations' in context of the Ontology/OntologySourceData/OwlKnowledgebase in order to be more specific.
@julesjacobsen
Copy link
Contributor Author

@cmungall / @jnguyenx can you comment on this further or merge this please. We can add the labels in another PR.

Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't had time to fully explore but broadly this looks good

@@ -18,12 +20,37 @@ public void testItAll() {
curies.put("MP", "http://purl.obolibrary.org/obo/MP_");
curies.put("NCBITaxon", "http://purl.obolibrary.org/obo/NCBITaxon_");

Map<String, List<String>> individuals = new LinkedHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should now have a thought about how to add additional information to an annotation. E.g. a weight or something.

Is it already possible to add negation?

@drseb
Copy link
Member

drseb commented Mar 20, 2017

I think it is good. Thanks. Can't test this right now.

@julesjacobsen
Copy link
Contributor Author

OK, cheers for looking. @drseb I think this should make it possible to do what you need. If not, open another ticket.

@julesjacobsen julesjacobsen reopened this Mar 20, 2017
@julesjacobsen julesjacobsen merged commit 933184c into monarch-initiative:master Mar 20, 2017
@drseb
Copy link
Member

drseb commented Mar 20, 2017

Thanks. I will test ASAP

Copy link
Contributor

@jnguyenx jnguyenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

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

Successfully merging this pull request may close these issues.

None yet

4 participants