Added option to select encoding of .bib file in parse_file() #395
Conversation
|
Thanks @himcraft for the PR. The code change looks good, but would you mind adding a few tests? I know that the change is rather trivial, thus the purposes of the tests would not be to verify if you change is correct (I am convinced that it is), but instead serve to prevent from regression, i.e., from unintentionally breaking this function in the future. Thanks! Edit: Also make sure to run linting ( |
|
Added |
MiWeiss
left a comment
MiWeiss
left a comment
There was a problem hiding this comment.
Thanks a lot for your commit.
Your test would certainly already be sufficient as a regression test. I still have left some minor comments, mostly to keep the test atomic and the test organisation (read file structure) consistent with the remainder of the test suite.
Afterwards, this will be ready to be merged 🚀 Thanks!
tests/test_encoding.py
Outdated
|
|
||
| def test_gbk(): | ||
| library = parse_file("tests/gbk_test.bib", encoding="gbk") | ||
| lines = writer.write(library).splitlines() |
There was a problem hiding this comment.
I would suggest not making the assertion on the "re-written" bibtex, but on the parsed entities instead.
e.g. assert library.entries[0]["author"] == ...
That way, we can keep the test independent of the writer.
tests/test_encoding.py
Outdated
| @@ -0,0 +1,17 @@ | |||
| """Testing the parse_file function.""" | |||
There was a problem hiding this comment.
can we re-name this file to test_entrypoint and the module docstring accordingly - there's likely to be added more tests to it eventually.
| @@ -0,0 +1,6 @@ | |||
| @ARTICLE{¿Èö2013, | |||
There was a problem hiding this comment.
Please move this file into a (new) tests/resources folder.
MiWeiss
left a comment
MiWeiss
left a comment
There was a problem hiding this comment.
lgtm!
Thanks a lot for your contribution. If there's any more problems or missing features you spot, please do not hesitate to report them.
2 participants
No description provided.