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 tutorial #99

Merged
merged 18 commits into from
Jul 30, 2021
Merged

Add tutorial #99

merged 18 commits into from
Jul 30, 2021

Conversation

gaurav
Copy link
Member

@gaurav gaurav commented May 2, 2021

This PR adds an "Introduction to phyx.js" tutorial that describes how the library is intended to be used. Since phyx.js doesn't have a standard workflow, I instead wrote this tutorial to look at how Phyx documents can be accessed as JSON documents, how phyx.js wrappers can help access parts of those documents, and how it can be used to convert the whole document into an OWL document.

I could write additional tutorial steps demonstrating how to use PhylogenyWrapper to recurse through all the nodes in a phylogeny, or PhylorefWrapper to calculate the default nomenclatural code for a phyloreference, but I suspect that's too in-the-weeds for what is intended to be a broad introduction.

I wrote the tutorial in Jupyter Notebook running an IJavascript kernel. The Jupyter Notebook itself is included in this PR, but since GitHub's rendering of this notebook sometimes fails unexpected, I think we should point people to the Markdown version of this file instead. This PR will also include this tutorial in the phyx.js documentation website at https://www.phyloref.org/phyx.js/manual/index.html.

Closes #98.

Installed a library that should allow us to execute Node.js code with it. Even if a Python Notebook is not the best way to store a tutorial (and I think it might be), this will allow me to organize my thoughts and the code-output format will be useful in recording what users should do and what the result is.
@gaurav gaurav marked this pull request as ready for review May 3, 2021 05:50
@gaurav gaurav requested a review from hlapp May 3, 2021 05:50
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.

Nice start. My major comment here is that I think demonstrating the use of the JSON package, which would include using JavaScript to navigate a Phyx file as a JSON file, is out of scope. (And part of the motivation for phyx.js is that people don't have to do that, isn't it?)

So I think that part should be mostly cut, save perhaps for a few (1-2) things (such as specifiers?) to demonstrate the added value phyx.js brings to inspecting the content of a Phyx file.

Second, the issue description (which BTW does IMO give the correct scope) says how to read a Phyx file, check it for validity, examine phyloreferences, phylogenies and specifiers, and describe how to convert the file into RDF. I do agree with that description, but I'm not sure each of these points is fully reflected in the tutorial. It's certainly not structured like this, and it's not clear why not, or what is gained by using a different structure.

As a more minor comment, consider for every place a variable includes the wrapped prefix, whether the fact that it's an instance of a wrapping class is important to carry through for the tutorial reader, or whether that's most motivated by you avoiding name clashes, i.e., needing to distinguish by name between the object passed in and the wrapped object instantiated from it. For example, isn't conceptually a wrappedPhyloref simply a phyloreference object, and a wrappedSpecifier simply a specifier object? For a tutorial, one that reads quicker due to fewer letters to discern is better.

@gaurav
Copy link
Member Author

gaurav commented May 5, 2021

So I think that part should be mostly cut, save perhaps for a few (1-2) things (such as specifiers?) to demonstrate the added value phyx.js brings to inspecting the content of a Phyx file.

Good point! Done in d80b0ef.

Second, the issue description (which BTW does IMO give the correct scope) says how to read a Phyx file, check it for validity, examine phyloreferences, phylogenies and specifiers, and describe how to convert the file into RDF. I do agree with that description, but I'm not sure each of these points is fully reflected in the tutorial. It's certainly not structured like this, and it's not clear why not, or what is gained by using a different structure.

After a few changes, I've ended up with the following structure:

  • Navigating a Phyx document using JSON
  • New section: Validating a Phyx document using JSON Schema
  • Navigating a Phyx document using phyx.js
    • Examining phyloreferences, taxonomic units and taxon concepts
    • New section: Examining phylogenies
    • Accessing citations
  • Converting a Phyx document into OWL
  • About this notebook

I think this works well, and is pretty good to the originally proposed order. What do you think?

As a more minor comment, consider for every place a variable includes the wrapped prefix, whether the fact that it's an instance of a wrapping class is important to carry through for the tutorial reader, or whether that's most motivated by you avoiding name clashes, i.e., needing to distinguish by name between the object passed in and the wrapped object instantiated from it. For example, isn't conceptually a wrappedPhyloref simply a phyloreference object, and a wrappedSpecifier simply a specifier object? For a tutorial, one that reads quicker due to fewer letters to discern is better.

True, but in this case the distinction is important since most of the wrapped objects don't entirely wrap the entire object, mainly to avoid writing a lot of boilerplate code: for example, PhylogenyWrapper doesn't reveal the underlying Newick string, since accessing it via phylogeny.newick || '' is pretty straightforward, and you can also access it from the wrapped phyloref via wrappedPhyloref.phyloref.newick if need be. The wrappers usually have methods to access information that you can't get directly from the underlying object, such as the list of internal and terminal labels or the taxonomic units associated with a label name. So the phylogeny/wrappedPhylogeny distinction comes from the phyx.js and Klados code where I started making that distinction.

Does that make sense? I could add a shorter version of the previous paragraph to the "Examining phyloreferences, taxonomic units and taxon concepts" section if it would help, but I think it might be too much detail for this tutorial. Alternatively, I could actually add the necessary boilerplate code to make all this work if that would be better (i.e. add a getter so that wrappedPhyloref.newick works correctly). Let me know what you think!

@gaurav gaurav requested review from hlapp and removed request for hlapp May 5, 2021 05:46
@gaurav
Copy link
Member Author

gaurav commented May 5, 2021

Based on our phone conversation, I'll delete the sections "Navigating a Phyx document as a JSON file" and "Validating a Phyx document using JSON Schema", which aren't directly relevant to a phyx.js tutorial. We should move the "Validating a Phyx document using JSON Schema" into the README file. Add some text to be a bit clearer on what the wrappers can do and cannot do, and then focus just on what the wrappers do (which is what the later sections do).

(Add a section at the end to say "Here's all the nice, useful things we can do with phyx.jx ... and here are some other things that you can't do (there are many data and attributes not accessible through phyx.js), but that's because it's pretty accessible directly in JSON").

@gaurav
Copy link
Member Author

gaurav commented May 13, 2021

@hlapp Here's an updated version of the tutorial as per our conversation from last week. Let me know what you think!

@gaurav gaurav requested a review from hlapp May 13, 2021 04:44
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.

Overall I like this rewrite. I've made a few inline edits, though I suspect you'll need to port them over to the Jupyter NB version (which is hard to make edits in using just GitHub).

tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
tutorial/Introduction to phyx.js.md Outdated Show resolved Hide resolved
@hlapp
Copy link
Member

hlapp commented Jul 23, 2021

One thing I forgot is to add date and author under the title.

@gaurav
Copy link
Member Author

gaurav commented Jul 27, 2021

@hlapp I've made the changes you requested, and fixed an issue in the tests where we hard-coded the Open Tree synthetic tree version (I'll fix this eventually in #101). Have a look, and let me know what you think!

@gaurav gaurav requested a review from hlapp July 27, 2021 03:12
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 9187772 into master Jul 30, 2021
@gaurav gaurav deleted the add-tutorial branch July 30, 2021 03:56
@gaurav gaurav mentioned this pull request Oct 15, 2021
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.

Write a tutorial for Phyx.js
2 participants