Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The attribute standard_relationship should be a collection #6

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Conversation

andrew2net
Copy link
Contributor

fixes #5

@andrew2net andrew2net requested a review from ronaldtse November 21, 2024 04:12
@ronaldtse
Copy link
Contributor

@andrew2net can you add the spec fixture for the failure so we don't encounter it again? You can just copy that file from rawbib. Thanks.

@andrew2net
Copy link
Contributor Author

andrew2net commented Nov 21, 2024

@ronaldtse I can add fixtures. But we need to create another test for this cases because lutaml-model creates collection automatically even there is no collection: true option. So round-trip test doesn't fall without the option. But using Ieee::Idams::Publication's data is tricky because we need to handle 3 value types. For example without collection: true option abstract's value can be:

  • nil if there is no any abstracts
  • ArticleAbstract instance when there is only one abstract
  • [ArticleAbstract] array of abstract instances if there are more than one abstract

With the option we always get an array and can handle it same and simplify code in the relaton-ieee.
I seems there are other attributes what need to be collection. I'll create new tests as soon as I find all of them.

@ronaldtse
Copy link
Contributor

@andrew2net I think this is a problem in lutaml-model for being implicit or "not caring" the actual model.

This just means that ArticleAbstract is clearly a collection because it could be 0, 1 and *, i.e. [0..*].

I think when collection: true is not given, then we must enforce there is 1 and only 1 instance. Ping @HassanAkbar .

@andrew2net
Copy link
Contributor Author

I think this is a problem in lutaml-model for being implicit or "not caring" the actual model.

@ronaldtse agree. If lutaml-model caries the actual model then the round-test will fall without collection option when an attribute is actually collection. So we won't need addition tests in this gem.

@ronaldtse
Copy link
Contributor

Copy link
Contributor

@ronaldtse ronaldtse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrew2net thank you, please feel free to merge!

@ronaldtse ronaldtse merged commit de59431 into main Nov 21, 2024
13 of 14 checks passed
@ronaldtse ronaldtse deleted the issue5 branch November 21, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The attribute standard_relationship should be a collection
2 participants