Closed Smund27 closed 1 year ago
Hi, Samuel and Stuart.
I have revised the Jupyter notebook based on the comments by Samuel. Here is the summary of the changes.
Link in first bullet point could be misunderstood as a real link. It should be explicitly referred to as an example input. --> Fixed.
I think each of these bullets should have an example of what each part of the string might look like as has been done above. --> Added a figure (from https://doi.org/10.1093/nar/gkv396). This figure is in public domain because I created it as part of my official gov duty, but I think it is too big with too many examples, so I’ll remake the figure later. So, consider this figure is a sort of place holder.
We are assuming they know what a CID is --> Added a short description on CID and added a link to the “Repository: PubChem” page in the book.
Might be good to have a definition of PUG-REST (it sounds ridiculous to a first time user) --> Mentioned that PubChem has multiple programmatic access routes and PUG-REST one of them. Both the acronym PUG and REST are defined in the text.
Volume spelt incorrectly in the 'Request volume Limitations' link --> Fixed.
Should there be a glossary to have definitions instead of having them in the text? --> Not sure which terms you are talking about, but in the revised version, I added links to some webpages if I think some extra explanation may be helpful. If there are specific items that you think should be included in the Glossary, please let me know.
In the returning images code it is unclear why the string becomes 'PNG?image...'- the way the string is built is obvious and logical up until that point. As soon as special characters get introduced (such as ?) first time users won't understand why it has changed. --> Added an explanation about how to specify optional parameters (in the PUG-REST syntax part). Also added a description of the optional parameters “image_size” and “record_type”.
Opening the file using a with statement is assuming a bit of Python knowledge. Should there be more comments around the with statement? --> Added an explanation about the "with open ... as ..." statement.
In the with statement should it be more explicit? Instead of 'f', explicitly write 'file'. --> Fixed.
In the numbered points for the 2 step process of finding other diverse conformers I don't think you need the 'and' at the end of the first point. --> Fixed.
There is no output from the final with statement, and also it is not explicit as to what is happening (see point above) --> Added a sentence what the last cell is supposed to do after the cell.
If the revised version looks okay with you, please close this issue. If you have comments on the other two PubChem PUG-REST notebooks, please let me know.
Thank you and have a good day.
Best,
Sunghwan,
Which page are you reviewing? https://iupac.github.io/WFChemCookbook/recipes/pubchem-pug-rest1.html
What is your ORCID https://orcid.org/0000-0001-5404-6934
Review I am reviewing this as a regular Python user who hasn't used APIs for a long time and who hasn't ever used PubChem PUG-REST requests. I also teach 2nd year chemists an introductory Python course and will be looking at it from their perspective.
General points
Overall
Easy to follow and understand, shows the main points of querying via IDs/descriptors etc. I think at points the code needs to be more explicit so first time Python users understand what each point in the code means. I also wonder if a glossary would be useful so that all explanations can be rolled into one.