Codecov Report

Merging #41 (8d6aa3f) into main (9d2dda0) will increase coverage by 0.18%. The diff coverage is 100.00%.

:exclamation: Current head 8d6aa3f differs from pull request most recent head dd8c4c2. Consider uploading reports for the commit dd8c4c2 to get more accurate results

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   95.44%   95.63%   +0.18%     
==========================================
  Files          20       20              
  Lines         813      825      +12     
==========================================
+ Hits          776      789      +13     
+ Misses         37       36       -1

Impacted Files	Coverage Δ
relaton/__init__.py	`100.00% <100.00%> (ø)`
relaton/serializers/bibxml/reference.py	`94.44% <100.00%> (+1.11%)`	:arrow_up:
relaton/tests/test_serializers.py	`99.25% <100.00%> (+0.03%)`	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

stefanomunarini commented 2 years ago

https://github.com/relaton/relaton-py/issues/44

strogonoff commented 2 years ago

I think this is a bit fragile. This assumes that doctype field has value "internet-draft". However,

We can’t be sure it will be a string that exactly matches "internet-draft" in lowercase
From what I see, the field is not even specified here: https://github.com/relaton/relaton-models/blob/main/models/BibliographicItem.lutaml

@ronaldtse @opoudjis what is the most reliable way to determine if a Relaton bibliographic item is an Internet Draft?

One option is doctype == "internet-draft", Stefano uses it here (I have doubts about it for abovementioned reasons)
Another option is checking if any docid has type == "Internet-Draft"
Any other reliable ways?

andrew2net commented 2 years ago

I think this is a bit fragile. This assumes that doctype field has value "internet-draft". However,

We can’t be sure it will be a string that exactly matches "internet-draft" in lowercase

I'm sure there is a case-insensitive way to compare strings.

From what I see, the field is not even specified here: https://github.com/relaton/relaton-models/blob/main/models/BibliographicItem.lutaml

The doctype should be specified in the BibliographicItem class. We need to update all the Relaton models according to grammar. @ronaldtse @opoudjis what is the most reliable way to determine if a Relaton bibliographic item is an Internet Draft?

One option is doctype == "internet-draft", Stefano uses it here (I have doubts about it for abovementioned reasons)

I think this is the most reliable way.

strogonoff commented 2 years ago

I'm sure there is a case-insensitive way to compare strings.

@andrew2net my review was directed to Stefano who didn’t use a case-insensitive way.

But also the problem is discoverability of the string “internet-draft” regardless of case sensitivity. We are yet to implement a solution for that.

ronaldtse commented 2 years ago

I don’t see any problems (besides a very minor style issue), but from what I understand we may need to expand this to cover other documents, not just I-Ds.

Correct, thanks @strogonoff .

relaton / relaton-py

Replace <target ..> tag with <format ..> #41

Codecov Report