URN needs to use urn_stage fully

[opoudjis]

This is a bug.

Pubid::Iso::Identifier.new(number: 200, stage: "CD", urn_stage: "30.20").urn = "urn:iso:std:iso:200:stage-30.00"

This is incorrect: it is supposed to be "urn:iso:std:iso:200:stage-30.20". That is in fact the point of passing the urn_stage to the identifier. The urn values take priority over the stage value, especially as the stage value does not include the substage.

[ronaldtse]

Yes this is indeed a bug. Thanks.

[mico]

@opoudjis urn_stage is deprecated, I will remove it. You should use only stage, for your case it would be:

Pubid::Iso::Identifier.new(number: 200, stage: Stage.new(abbr: :CD, harmonized_code: "30.20").urn

[opoudjis]

@mico The point of #102 was so that I would not be forced to invoke a separate Stage constructor when initialising the identifier.

It turns out I have to invoke the Stage constructor after all, in order to include harmonized_code. So there was no point removing the Stage constructor after all.

I am closing this ticket, because enough time has been squandered on refactoring the API.

[opoudjis]

I am not closing this ticket, because you are crashing if I try to create a stage with a harmonised code for an amendment.

Pubid::Iso::Identifier.new(number: 200, amendments: [{:number=>"1", stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

I have pushed a PR to sidestep this issue. If this does not fit the framework you have used in transform, then change it.

[mico]

I am not closing this ticket, because you are crashing if I try to create a stage with a harmonised code for an amendment.

Pubid::Iso::Identifier.new(number: 200, amendments: [{:number=>"1", stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

I have pushed a PR to sidestep this issue. If this does not fit the framework you have used in transform, then change it.

I don't understand why it's "crashing". Could you give have more details on error?
We already have a test for it here:

pubid-iso/spec/pubid_iso/identifier/create_new_identifier_spec.rb

Lines 77 to 87 in 9956b9a

	context "at stage PRF" do
	let(:stage) { Stage.new(harmonized_code: "50.00", abbr: :PRF) }
	it "renders separate stage for PubID" do
	expect(subject.to_s(with_prf: true)).to eq("ISO/PRF #{number}")
	end
	it "renders separate numeric stage for URN" do
	expect(subject.urn).to eq("urn:iso:std:iso:#{number}:stage-50.00")
	end
	end

And it's not "crashing".
Your change in pull request don't make sense, in transformer we never get Stage object.

[opoudjis]

Artur, your test cases simply do not correspond with what I am telling you I am doing. It is crashing, and it is a legal call.

I need to create a PubId for an amendment.

The amendment has a harmonized code.

Therefore, the ONLY place I can specify the harmonized code, as you have explicitly told me, is in a Stage call, and the only place I can put the Stage call correctly is in the Amendment specification.

I am generating an identifier as:

Pubid::Iso::Identifier.new(number: 200, 
  amendments: [{:number=>"1", 
  stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

You see that the Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20") call is INSIDE of amendements: [...]. That is correct, because the stage is the stage of the amendment.

You explicitly allow Stage constructors inside supplements in your documentation, as values of stage, along with strings:

https://github.com/metanorma/pubid-iso/blob/main/lib/pubid/iso/supplement.rb

    # @param stage [Stage, Symbol, String] stage, e.g. "PWI", "NP", "50.00", Stage.new(abbr: :WD)
    # @param publisher [String] publisher, e.g. "ISO", "IEC"
    # @param edition [Integer] edition, e.g. 1, 2, 3
    # @param iteration [Integer] iteration, e.g. 1, 2, 3
    # @see Pubid::Core::Supplement for other options
    def initialize(stage: nil, publisher: nil, edition: nil, iteration: nil, **args)
      super(**args)
      @stage = stage.is_a?(Stage) ? stage : Stage.parse(stage) if stage
      @publisher = publisher.to_s
      @edition = edition&.to_i
      @iteration = iteration&.to_i

      if @iteration && @stage.nil?
        raise Errors::PublishedIterationError.new("cannot assign iteration to published supplement")
      end
    end

You explicitly allow:

 # @param stage [Stage, Symbol, String] stage, e.g. "PWI", "NP", "50.00", Stage.new(abbr: :WD)

@stage = stage.is_a?(Stage) ? stage : Stage.parse(stage) if stage

Therefore, your assertion "Your change in pull request don't make sense, in transformer we never get Stage object." is incorrect.

However, when I generate:

Pubid::Iso::Identifier.new(number: 200, 
  amendments: [{:number=>"1", 
  stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

the call amendments processing makes to convert_stage() aborts, because your transformer code does indeed presupposes that amendments can only contain a string value.

If this call is incorrect:

Pubid::Iso::Identifier.new(number: 200, 
  amendments: [{:number=>"1", 
  stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

then you need to tell me how to specify both stage abbreviation and harmonised code for an amendment instead. But your documentation of supplements, which amendments and corrigenda are a subclass of, is explicit that my call is legal.

Your documentation hints that I should be doing stage: "50.00". That is news to me, and I am not confident that I should be dissociating abbreviation from code, given the ambiguity of "PRF".

[ronaldtse]

And it's not "crashing".

irb(main):001:0> Pubid::Iso::Identifier.new(number: 200, amendments: [{:number=>"1", stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])
irb(main):002:0* 
pubid-iso/lib/pubid/iso/stage.rb:53:in `initialize': undefined method `to_sym' for #<Pubid::Iso::Stage:0x0000000109d4d640 @abbr="CD", @harmonized_code=#<Pubid::Iso::HarmonizedStageCode:0x0000000109d4d4b0 @substage="20", @stage="30">> (NoMethodError)

        raise Errors::StageInvalidError, "#{abbr} is not valid stage" unless STAGES.key?(abbr.to_sym)
                                                                                             ^^^^^^^
Did you mean?  to_s

It does crash.

[ronaldtse]

In any case, the issue with amendments is to be handled in #130 .

[mico]

And it's not "crashing".

irb(main):001:0> Pubid::Iso::Identifier.new(number: 200, amendments: [{:number=>"1", stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])
irb(main):002:0* 
pubid-iso/lib/pubid/iso/stage.rb:53:in `initialize': undefined method `to_sym' for #<Pubid::Iso::Stage:0x0000000109d4d640 @abbr="CD", @harmonized_code=#<Pubid::Iso::HarmonizedStageCode:0x0000000109d4d4b0 @substage="20", @stage="30">> (NoMethodError)

        raise Errors::StageInvalidError, "#{abbr} is not valid stage" unless STAGES.key?(abbr.to_sym)
                                                                                             ^^^^^^^
Did you mean?  to_s

It does crash.

By doing:

Identifier.create(number: 200, amendments: [{:number=>"1", stage: Pubid::Iso::Stage.new(abbr: :CD, harmonized_code: "30.20")}])

Using latest pubid-iso It's not crashing anymore.

[ronaldtse]

Thank you @mico. Can you add this example to the README too? Thanks.