Skip to content
This repository was archived by the owner on Dec 21, 2025. It is now read-only.

Conversation

@pmartin23
Copy link
Contributor

@pmartin23 pmartin23 commented Oct 10, 2018

closes #157

@reece
Copy link
Member

reece commented Oct 10, 2018

Looks good so far! Would you please also add a test?

Specifically, in tests/test_eutils_xmlfacades_pubmedarticle.py, add a new test function for pubmed id 29915538. You can use the previous test as a template. At the very least, the test should ensure that the abstract method works with something like assert "Semaglutide" in pa.abstract.

You can run the tests this with make test.

(Apologies if this is stuff you already know. You previous said that this was your first PR so I was assuming tips would help.)

Thanks for the contribution!

@pmartin23
Copy link
Contributor Author

Yes, of course - I'll add in the test now.
Please don't apologise - those tips are super useful - thank you!

@reece reece merged commit af96097 into biocommons:master Oct 10, 2018
@reece
Copy link
Member

reece commented Oct 10, 2018

Thanks @pmartin23! Congrats on your first PR!

I "squashed and merged" the commits so that these appear as a single commit. You may want to git pull origin master to sync your repo.

reece pushed a commit that referenced this pull request Oct 10, 2018
…abstract text (#158)

* fix error that occurrs when AbstractText element contains formatting tags
* add test to cover edge case referenced in #157
reece added a commit that referenced this pull request Oct 10, 2018
@pmartin23
Copy link
Contributor Author

How exciting! You're welcome, and thanks for your help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Edge case results in error being thrown when extracting abstract text in medlinecitation.py

2 participants