Errors in ID generation

opoudjis commented 2 years ago

I think there is a clear misunderstanding in how this gem is to be used.

The tests for this gem round trip from parsing ISO IDs, to constructing IDs based on the parse.

The correct way of testing this gem is to construct identifier class instances from parameters, and then compare the results to the defined exemplars.

The reason the current tests are flawed are:

There are several attribute values that are not rendered, e.g. "IS"; they are not parsed from ISO IDs, but are present when an identifier is generated from a constructor
There are several attribute values that are transformed for rendering; e.g. language values
Users are being given no guidance in how to use constructors, which is an acute problem given that the documentation is so sparse.

I have attempted to use this gem with the identifiers generated by metanorma-iso in its testing. None of them are correct. These must be fixed.

Pubid::Iso::Identifier.new() parameters, expected value, generated value:

ISO. 17301-1. 2016. AMENDMENT 1. 2017.

irb(main):005:0> Pubid::Iso::Identifier.new(number: 17301, part: 1).to_s
=> " 17301-1"
irb(main):011:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "NP"}]).to_s
=> "ISO 17301-1:2016/NP Amd :1(en)"
irb(main):012:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "NP",
 year: 2017}]).to_s
gems/pubid-core-0.2.2/lib/pubid/core/supplement.rb:6:in `initialize': unknown keyword: :year (ArgumentError)                                                                                                                                     
        from gems/pubid-iso-0.1.10/lib/pubid/iso/supplement.rb:6:in `initialize'          
        from gems/pubid-core-0.2.2/lib/pubid/core/identifier.rb:11:in `new'               
        from gems/pubid-core-0.2.2/lib/pubid/core/identifier.rb:11:in `block in initialize'                                                                                                                                                      
        from gems/pubid-core-0.2.2/lib/pubid/core/identifier.rb:10:in `map'
        from gems/pubid-core-0.2.2/lib/pubid/core/identifier.rb:10:in `initialize'
    from gems/pubid-iso-0.1.10/lib/pubid/iso/identifier.rb:10:in `initialize'
    from (irb):12:in `new'
    from (irb):12:in `<main>'
    from gems/irb-1.4.1/exe/irb:11:in `<top (required)>'

[x] #78
[x] #79
[x] #69 (Must allow rendering language differently between "E" and "en")
[x] #48 (Should not have to look up the state abbreviation based on the urn_stage, that should be the gem's job)
[x] #80

ISO 17301-1 Amd 1 DIS

irb(main):007:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "DIS"
}]).to_s
=> "ISO 17301-1:2016/DIS Amd :1(en)"

This is wrong. The correct output is (depending on rendering options of #69):

"ISO 17301-1:2016/DAmd 1(en)" (again, (en) or (E) depends on rendering options, the latter should be default)
"ISO 17301-1:2016/DAM 1(en)"

ISO 17301-1 Amd 1 (stage = published)

irb(main):017:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "IS"}
]).to_s
=> "ISO 17301-1:2016/IS Amd :1(en)"
irb(main):018:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "IS"}
]).urn
=> "urn:iso:std:iso:17301:-1:stage-60.00:amd:1:v:en"
irb(main):019:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1}]).to_s
=> "ISO 17301-1:2016/Amd :1(en)"
irb(main):020:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1}]).urn
=> "urn:iso:std:iso:17301:-1:amd:1:v:en"

[x] #81

Notice that the Identifier object differentiates the "no-stage" vs "IS stage" (published) in the URN, the rendering of the "IS stage" identifier is wrong.

When the stage is "IS", the output should be "ISO 17301-1:2016/Amd 1(en)", not "ISO 17301-1:2016/IS Amd :1(en)".

This behavior also applies to "Cor".

ISO 17301-1/CD Cor 3

Generated: "ISO 17301-1/CD Cor :3" Correct: "ISO 17301-1/CD Cor 3"

ISO 17301-1, FDIS stage, Cor 3 => "ISO 17301-1/FD Cor 3"

Generated: "ISO 17301-1/FD Cor :3" Correct: "ISO 17301-1/FDCor 3"

Stage iteration identifiers in Amd or Cor

Stage iteration is supported in the normal pubid.

irb(main):024:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, stage: "WD", iteration: 2).to_s
=> "ISO/WD 17301-1.2:2016(en)"

But we should throw an error when given an iteration number in "IS" stage:

irb(main):025:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, stage: "IS", iteration: 2).to_s
=> "ISO/IS 17301-1.2:2016(en)"

This is filed in #82.

The iteration number is also not supported in Amd or Cor.

irb(main):026:0> Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage:"WD", 
iteration:2}]).urn
gems/3.1.0/gems/pubid-core-0.2.2/lib/pubid/core/supplement.rb:6:in `initialize': unknown keyword: :iteration (ArgumentError)

[x] #83

TR and TS documents with subparts

irb(main):030:0> Pubid::Iso::Identifier.new(number: 17301, part: "1-1", publisher: "ISO", language: "en", year: 2016, type: "TS", copublisher: ["IEC","IEEE"]).to_s
=> "ISO/IEC/IEEE TS 17301-1-1:2016(en)"

This output is correct.

TR and TS documents with DIS or FDIS stages

irb(main):031:0> Pubid::Iso::Identifier.new(number: 17301, part: "1-1", publisher: "ISO", language: "en", year: 2016, type: "TS", stage: "DIS", copubli
sher: ["IEC","IEEE"]).to_s
=> "ISO/IEC/IEEE TS DIS 17301-1-1:2016(en)"

This should have been "ISO/IEC/IEEE DTS 17301-1-1:2016(en)" (instead of "TS DIS", it is "DTS").

irb(main):032:0> Pubid::Iso::Identifier.new(number: 17301, part: "1-1", publisher: "ISO", language: "en", year: 2016, type: "TS", stage: "FDIS", copublisher: ["IEC","IEEE"]).to_s
=> "ISO/IEC/IEEE TS FDIS 17301-1-1:2016(en)"

This should have been "ISO/IEC/IEEE FDTS 17301-1-1:2016(en)" (instead of "TS FDIS", it is "FDTS").

The same applies to type: "TR".

[x] #84

ISO Directives

Right now:

irb(main):040:0> Pubid::Iso::Identifier.parse("ISO DIR 1").type
=> nil
irb(main):041:0> Pubid::Iso::Identifier.parse("ISO DIR 1").dir
=> "DIR"

Why don't we use the type attribute to hold DIR, but a new dir attribute?

[x] #85

ronaldtse commented 2 years ago

I have re-checked all the examples provided and revised the original post in detail. Some of the original issues were not accurate.

@mico can you help check the examples above and create issues as necessary? Thanks.

ronaldtse commented 2 years ago

@mico I've updated the ticket again and split the work out. Please review the associated tickets.

ronaldtse commented 2 years ago

Amendment/Corrigendum

We need a simple way of adding an Amendment / Corrigendum.

I'm surprised that this doesn't work:

Pubid::Iso::Identifier.new(number: 17301, part: 1, publisher: "ISO", language: "en", year: 2016, amendments: [{number: 1, stage: "NP"}]).to_s
lib/pubid/iso/renderer/base.rb:58:in `block in render_amendments': undefined method `render_pubid' for {:number=>1, :stage=>"NP"}:Hash (NoMethodError)

      amendments.sort.map { |amendment| amendment.render_pubid(opts[:stage_format_long], opts[:with_date]) }.join("+")
                                                 ^^^^^^^^^^^^^
    from lib/pubid/iso/renderer/base.rb:58:in `map'
    from lib/pubid/iso/renderer/base.rb:58:in `render_amendments

Where's the documentation on how to create an Amendment identifier?

Stage entry

Users won't bother creating a class when creating an identifier. If the stage given is not a class, look it up for them.

Pubid::Iso::Identifier.new(number: 17301, part: "1-1", publisher: "ISO", language: "en", year: 2016, type: "TS", stage: "DIS", copublisher:
 ["IEC","IEEE"]).to_s
lib/pubid/iso/identifier.rb:43:in `initialize': undefined method `abbr' for "DIS":String (NoMethodError)

        if stage.abbr == "IS" && iteration                 
                ^^^^^                                      
        from (irb):6:in `new'                              
        from (irb):6:in `<main>'

ronaldtse commented 2 years ago

I will create new tickets for these remaining issues.

ronaldtse commented 2 years ago

The remaining tasks are :

100
101
102

Closing this one for now.

metanorma / pubid-iso