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

modify ebi_biomes.csv #736

Closed
wants to merge 6 commits into from
Closed

modify ebi_biomes.csv #736

wants to merge 6 commits into from

Conversation

kaiiam
Copy link
Contributor

@kaiiam kaiiam commented Mar 22, 2019

Addressing PR #735 and issue #672 I have proposed the following changes. Please merge if correct or modify.

pbuttigieg and others added 2 commits March 21, 2019 18:02
Addressing  PR #735 and issue  #672 I have proposed the following changes. Please merge if correct or modify.
@kaiiam kaiiam requested a review from cmungall March 22, 2019 06:14
Copy link
Member

@pbuttigieg pbuttigieg left a comment

Choose a reason for hiding this comment

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

+1 for the link rather than the pseudopath

@cmungall
Copy link
Member

much better!

I need to check if we need to uuencode spaces in the TSV, or whether this is taken care of at the OWL conversion level - can you post a snippet of the generated OWL?

@kaiiam
Copy link
Contributor Author

kaiiam commented Mar 22, 2019

compiling the ebi_biomes owl, the class renders as:

    <!-- http://purl.obolibrary.org/obo/ENVO_3100035 -->

    <Class rdf:about="http://purl.obolibrary.org/obo/ENVO_3100035">
        <rdfs:subClassOf rdf:resource="http://purl.obolibrary.org/obo/ENVO_01000252"/>
        <rdfs:subClassOf>
            <Restriction>
                <onProperty rdf:resource="http://purl.obolibrary.org/obo/RO_0002507"/>
                <someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/ENVO_00000488"/>
            </Restriction>
        </rdfs:subClassOf>
        <obo:IAO_0000115 rdf:datatype="http://www.w3.org/2001/XMLSchema#string">A freshwater lake biome which is determined by a glacial lake. </obo:IAO_0000115>
        <oboInOwl:hasDbXref rdf:datatype="http://www.w3.org/2001/XMLSchema#string">https://www.ebi.ac.uk/metagenomics/browse?lineage=root:Environmental:Aquatic:Freshwater:Ice:Glacial lake</oboInOwl:hasDbXref>
        <oboInOwl:hasNarrowSynonym rdf:datatype="http://www.w3.org/2001/XMLSchema#string">Glacial lake</oboInOwl:hasNarrowSynonym>
        <rdfs:label rdf:datatype="http://www.w3.org/2001/XMLSchema#string">glacial lake biome</rdfs:label>
    </Class>
</rdf:RDF>

@cmungall is this what you were expecting?

Should the space within the EBI path instead by handeled by a %20 instead of how I currently have it? i.e. http://www.w3.org/2001/XMLSchema#string">https://www.ebi.ac.uk/metagenomics/browse?lineage=root:Environmental:Aquatic:Freshwater:Ice:Glacial%20lake

@cmungall
Copy link
Member

cmungall commented Mar 23, 2019 via email

@pbuttigieg
Copy link
Member

Are we good to go with this merge @cmungall ?

@kaiiam
Copy link
Contributor Author

kaiiam commented Apr 25, 2019

Yes @cmungall if you don't mind weighing in, I'm unclear if I can / should change the this as it has the changes to envo-edit.owl from commit 2f04d9d, which @pbuttigieg merged in in another PR. Will git figure out that that commit has already been made and it will just ignore those changes? Or do we need to kill this PR and make my changes again. I don't know how/if you can get rid of specific commits on a branch.

This reverts commit 2f04d9d.
@kaiiam
Copy link
Contributor Author

kaiiam commented Apr 25, 2019

I wanted to try and figure it out myself I ran git revert --strategy resolve <commit> which worked to get rid the unwanted commits, and now only the correct file is changed, however now were getting the This branch has conflicts that must be resolved issue. I tried running the instructions to fix it via the command line but I couldn't get that to work. Sorry if I made it worse, I just thought that removing changes to envo-edit.owl would fix the issue.

@cmungall
Copy link
Member

Sorry not sure how best to resolve this.. if it isn't too much work, can you just do the dumb git thing. Copy the csv somewhere safe, close this PR, start a new one, and copy the csv over...?

@kaiiam
Copy link
Contributor Author

kaiiam commented Apr 25, 2019

Ya, no worries I suspected it may come to that. I'll do it that way, close this PR and make a new one.

@pbuttigieg
Copy link
Member

It's strange, I'm seeing something like this over at SDGIO too : ghost commits emerging in new branches, originating from branches which were previously merged and deleted. I seem to be the common denominator here, so will also try to learn more about this. Perhaps it's a git/cygwin thing.

@pbuttigieg
Copy link
Member

Yeah, conflicts have arisen due to a previous merge. Best to redo this, after I fix @rduerr PR #759

@pbuttigieg
Copy link
Member

@kaiiam all done with PR #759 - will you be working on this in the next day or so? If not, then hold off on creating a new branch so other PRs can be formed without causing more conflicts.

@kaiiam
Copy link
Contributor Author

kaiiam commented Apr 26, 2019

Sure let me know when you finish, and I'll restart this PR.

@pbuttigieg
Copy link
Member

pbuttigieg commented Apr 27, 2019

Sure let me know when you finish, and I'll restart this PR.

Okay @kaiiam, all done. I'll wait for your new PR to be merged before starting any more.

kaiiam added a commit that referenced this pull request Apr 28, 2019
Due to merge conflicts, I will remake  remake PR #736 here in to address PR #735 and issue #672.
@kaiiam
Copy link
Contributor Author

kaiiam commented Apr 28, 2019

Changes re committed in PR #763.

@kaiiam kaiiam closed this Apr 28, 2019
pbuttigieg added a commit that referenced this pull request Apr 28, 2019
@pbuttigieg pbuttigieg deleted the kaiiam-patch-2 branch June 2, 2020 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants