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

Migrate to lutaml-model #90

Draft
wants to merge 32 commits into
base: main
Choose a base branch
from
Draft

Migrate to lutaml-model #90

wants to merge 32 commits into from

Conversation

andrew2net
Copy link
Contributor

No description provided.

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

@ronaldtse
Copy link
Contributor

@andrew2net is this in progress?

@ronaldtse ronaldtse changed the title Shale integration Migrate to lutaml-model Dec 18, 2024
Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

@andrew2net
Copy link
Contributor Author

andrew2net commented Dec 31, 2024

@ronaldtse yes, this is in progress. I can finish XML serialization in a week. But there is the blocking issue metanorma/metanorma-standoc#952

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

@@ -31,6 +31,12 @@ def initialize(**args) # rubocop:disable Metrics/CyclomaticComplexity
@formatted_address = args[:formatted_address] unless args[:city] && args[:country]
end

def ==(other) # rubocop:disable Metrics/AbcSize
Copy link
Contributor

Choose a reason for hiding this comment

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

Lutaml::Model supports equivalence checks of attributes

def initialize(organization: nil, name: nil, description: [])
@name = name
@organization = organization
@description = description
end

def ==(other)
Copy link
Contributor

Choose a reason for hiding this comment

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

As above.

@ronaldtse
Copy link
Contributor

@andrew2net for the migrated models, why not convert all the model elements like docidentifier to lutaml-model directly instead of using the model ... keyword to maintain separate serialization and data models? They can be defined in the same model unless there is something very special that is done...

@andrew2net
Copy link
Contributor Author

@ronaldtse there are some reasons to have separate models

  • RelatonXML has 2 variants, bibitem and bidata. With data separated from mappings we can use Bibitem.to_xml(item) or Bibdata.to_xml(item) we can have output what we need.
  • Except RelatonXML we require rendering BibXML. So we can use different renderers for same data BibXML.to_xml(item) or Bibitem.to_xml(item)
  • We also have asciibib and bibtex formats that lutaml-model doesn't support. I'm certain we shouldn't have all that in one class.
  • Many elements have extra functionality. For example, Docidentifier has remove_part, remove_date, all_parts. I think it's better to separate Relaton data model and renderers.
  • In some cases, Relaton uses collections with extra functionalities instead of Arrays. For example, TitleCollection and RelationCollection.

@ronaldtse
Copy link
Contributor

Thank you @andrew2net for the insights. These are all valid concerns and use cases that lutaml-model does not handle well currently.

For items 1 and 2, they are about "alternative serializer definitions". I'm hoping to address it by an extension of this task, which is about a proposed Transform class:

For item 3, it is about additional serialization formats that lutaml-model does not support (asciibib, bibtex) and likely will not support. We need to find a way to allow users to define their own serializer formats in lutaml-model.

For item 4, it is about methods that are only about the "data model" (attribute and type definitions), and are irrelevant to "serializers" (the xml do... etc blocks). Right now, the arrangement of lutaml-model mixing "data model" (attributes) and "serializer" (xml do..) is not ideal. I still think that item 4 can be implemented in lutaml-model models today.

For item 5, it is about handling collections that are more complex than arrays, e.g. they need additional methods and have serialization formats. We have this task:

I will file the other issues in lutaml-model as enhancements. Thank you for the details.

@andrew2net
Copy link
Contributor Author

@ronaldtse
Since bibitem and bibdata are just a subsets of whole Relaton data model, and have same classes relations, for 1 we can create different root mappings using current lutam-model functionality. These mappings just exclude some attributes and have different root element. For example bibitem doesn't have ext flavor data, and bibdata doesn't have id attribute.

For 2 it needs to remap classes relationships. Since 2 is XML, the model-to-model transformation could work. Is that functionality expected soon?

For 3 we can use decorator pattern to implement serialization that lutaml-model doean't support.

Concernig 4, we can implement all the extra methods in the mapping classes. It's against single responsibility principle, but it's doeable.

For 5 (collections) we need to implement lutaml/lutaml-model#215. When is it expected?

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

@ronaldtse
Copy link
Contributor

@andrew2net wow I really like the latest changes! As it was well said before — a developer’s worth is characterized by not how much code is added, but how much removed! 👍👍

@andrew2net
Copy link
Contributor Author

@ronaldtse now it needs to add functionality that we had in separated models. I think this is a good time to review the functions and decide which need to be kept.

@andrew2net
Copy link
Contributor Author

@ronaldtse until the lutaml-model to lutaml-model transformation is ready I don't see another way but to use an external model for Bibitem and Bibdata mappings.

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

@andrew2net
Copy link
Contributor Author

@ronaldtse I think we need to versioning data models. We already have schema-version attribute, but we also need to be able to parse previous formats. For example, if the grammar is updated and a new relaton-bib gem with the update is released, the new gem should be able to read datasets created with the previous version.

We can create new classes for each new version, inherit everything from the previous version, and add changes. To convert data from one class to another the lutaml-model to lutaml-model transformation could be used. But if the transformation is far from implementation I can map data between classes.

What do you think?

@ronaldtse
Copy link
Contributor

ronaldtse commented Feb 7, 2025

@andrew2net I completely agree with your statements for functionality:

  1. We can create new classes for each new version, inherit everything from the previous version, and add changes.
  2. To convert data from one class to another the lutaml-model to lutaml-model transformation could be used.

We currently have this task:

But it is not well elaborated.

Can you give some examples in that issue so we can complete the requirements on how we can manage parsing / manipulating / migrating the model versions?

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unre...
Error: unrecognized cop Performance/CaseWhenSplat found in .rubocop.yml, unrecognized cop Performance/Count found in .rubocop.yml, unrecognized cop Performance/Detect found in .rubocop.yml, unrecognized cop Performance/FlatMap found in .rubocop.yml, unrecognized cop Performance/ReverseEach found in .rubocop.yml, unrecognized cop Performance/Size found in .rubocop.yml, unrecognized cop Performance/StringReplacement found in .rubocop.yml

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.

2 participants