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

Add additional example file: Fisher et al, 2007 #278

Merged
merged 38 commits into from
Mar 8, 2023

Conversation

gaurav
Copy link
Member

@gaurav gaurav commented Nov 30, 2022

This PR restores Fisher et al, 2007 from previous versions of Klados so that we can have examples of phyloreferences with specimens so we can make sure that Klados supports them. Closes #232.

It also includes some minor fixes that were discovered in creating this example file:

  • We previously had separate "Primary reference phylogeny" and "Reference phylogeny" citations for phylogenies. This has now been replaced with a single "Source" citation as per the Phyx manuscript.
  • Added the phyloref definition to the CSV export.
  • Klados attempted to read the default nomenclatural code from the defaultNomenclaturalCodeURI field from the Phyx file, but the specification says this field should be called defaultNomenclaturalCodeIRI. Fixed.

The backend is down (I'm working on fixing it), but I have confirmed that this works locally on my computer:

Screenshot 2023-03-05 at 1 23 22 AM

@gaurav
Copy link
Member Author

gaurav commented Dec 13, 2022

Aha, I've figured out what the problem is with Fisher et al, 2007: the phylogeny nodes aren't being notated with any nomenclatural code. This is because phyx.js uses the following logic to determine how to choose a nomenclatural code:

https://github.com/phyloref/phyx.js/blob/d3623e3a97b3d46747295dd826b735aab7ed4f16/test/nomenclatural-codes.js#L116-L124

I should be able to use defaultNomenclaturalCodeIRI to set these, but (1) I'm curious as to why the second part of that algorithm didn't fix this as it does for brochu_2003.json -- I hope it's not because the occurrence entries confuse it!, and (2) I wonder why we didn't add a per-phylogeny field for storing the nomenclatural code -- that might be a neater solution than a per-document defaultNomenclaturalCodeIRI field?

@gaurav gaurav force-pushed the add-additional-example-files branch from d2c4b66 to 26a239f Compare March 5, 2023 07:01
@gaurav
Copy link
Member Author

gaurav commented Mar 5, 2023

Moving content from the PR description into an issue so we document some fixes:

Outstanding issues with Fisher et al:

  • Phyloref Exodictyon doesn't resolve because it appears to use the type (species?) of genus Exodictyon as an internal specifier, in addition to specimen Wall 2527, Fiji (UC), which is identified elsewhere in the publication as Exodictyon incrassatum. What should we match this type to?
    • This means that the type species of the genus is included (even if not present in the phylogeny). So we should annotate Exodictyon incrassatum as Exodictyon, and then both specifiers will resolve.
  • Phyloref Leucophanella doesn't resolve because it includes an internal specifier, internal Specifier: Type: S. revolutus Dozy & Molk., which isn't included in the phylogeny (or anywhere else in the paper). I've deleted it and mentioned the deletion in the curator comments.

@gaurav gaurav marked this pull request as ready for review March 5, 2023 07:09
@gaurav gaurav changed the title Add additional example files Add additional example file: Fisher et al, 2007 Mar 5, 2023
@gaurav gaurav requested a review from hlapp March 5, 2023 07:25
Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

👍

@gaurav gaurav merged commit 4a1c42c into master Mar 8, 2023
@gaurav gaurav deleted the add-additional-example-files branch March 8, 2023 05:20
gaurav added a commit that referenced this pull request Sep 23, 2024
…laturalCodeURI

I fixed some references to defaultNomenclaturalCodeURI as part of PR #278, but I also missed others. This PR replaces those with `defaultNomenclaturalCodeIRI` as per the Phyx.js paper: https://doi.org/10.7717/peerj.12618/table-1
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.

Add additional example files
2 participants