Fix load scopus csv authors affiliations#59
Fix load scopus csv authors affiliations#59tleedepriest wants to merge 6 commits intoNLeSC:masterfrom
Conversation
tleedepriest
commented
Aug 29, 2023
tleedepriest
commented
- added conftest file that supports simple tests on all docs in doc set
- made scopus_csv more consistent with scopus source (returning None as opposed to empty list and vice versa)
- add support for authors with multiple affiliations
- add support for current CSV file download and older version
- add tests for both versions and break tests out into independent tests.
…a from scopus. Make doc attributes consistent with scopus source.
|
Hi Travis. Thank you very much this contribution. Fixing the ability to load Scopus CSV files is a highly requested feature. I am happy to see that this PR is well documented and has thorough testing. I have made some (small) comments on the code. |
Not sure how to look at the comments? Are you sure they are there or am I missing something? |
| auths_id = auths_id[:-1] | ||
|
|
||
| auths_ids = auths_id.split(";") | ||
| auths_ids = [auth_id.lstrip().rstrip() for auth_id in auths_ids] |
There was a problem hiding this comment.
.lstrip().rstrip() can just become .strip()
| """ | ||
| auths_id = self.entry.get("Author(s) ID") | ||
|
|
||
| if auths_id == "": |
There was a problem hiding this comment.
This can just become if auths_ids:. If, in the future, the scopus format changes again then self.entry.get("Author(s) ID") could return None. The if auths_ids: will catch both empty strings "" and Nones.
|
|
||
| def pytest_generate_tests(metafunc): | ||
| path = os.path.dirname(__file__) + "/resources/scopus.csv" | ||
| docs = load_scopus_csv(path) |
There was a problem hiding this comment.
Will this call load_scopus_csv for ever single test, even those that have nothing to do with scopus? Would it be possible to move it to inside the if statement?
| path = os.path.dirname(__file__) + "/resources/scopus.csv" | ||
| docs = load_scopus_csv(path) | ||
| if "doc" in metafunc.fixturenames: | ||
| metafunc.parametrize("doc", docs) |
There was a problem hiding this comment.
Would it be possible to rename this from doc to scopus_doc? It the future, such a mechanism would be use to also use for all tests of the other data source backends.
Also, should it not be parametrize("doc", docs[0])?
|
Hi. You were right. They should be visible now! |
2 participants
