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

MathML parsing using LutaML-Model #2

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Conversation

suleman-uzair
Copy link
Member

This PR incorporates MathML parsing using LutaML-Model gem.

closes #1

lib/mml/parser/math.rb Outdated Show resolved Hide resolved
@@ -1,20 +1,20 @@
# frozen_string_literal: true

require "lutaml/model"
module Mml
class Maction < Lutaml::Model::Serializable
Copy link
Contributor

Choose a reason for hiding this comment

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

This is exactly how it should be 👍

In the future for Lutaml::Model the namespace should indeed be added when we generate these classes. FYI @HassanAkbar

@ronaldtse
Copy link
Contributor

@suleman-uzair I know you are trying out multiple methods to make the lutaml-model definition simpler — for any complexity or improvements, can you file issues at lutaml-model so we can improve it? Thanks!

@suleman-uzair
Copy link
Member Author

can you file issues at lutaml-model so we can improve it?

Absolutely, @ronaldtse, just FYI, I’ve already logged the bugs, and if I have any improvements or suggestions, I’ll make sure to create issues for those too.

@ronaldtse
Copy link
Contributor

Thanks, whatever ideas/patterns/templates that will help users of lutaml-model would be very helpful!

@@ -2,6 +2,29 @@

module Mml
module Configuration
# Sharing supported tags between models to avoid duplication
# TODO: Find a better way to do this
Copy link
Contributor

Choose a reason for hiding this comment

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

@suleman-uzair @HassanAkbar how about making a separate Serializable class that you can put in these shared attributes (and mappings?), then there is a mechanism to incorporate them into the models you want?

Copy link
Member Author

Choose a reason for hiding this comment

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

how about making a separate Serializable class that you can put in these shared attributes (and mappings?)

@ronaldtse, Sure. I was thinking the same, I’ll do that once the models are finalized.

Copy link
Contributor

Choose a reason for hiding this comment

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

@suleman-uzair I believe inheritance allows inheriting attributes but the serialization configurations (mappings) are not inherited. You will need @HassanAkbar 's help here.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ronaldtse Yes, I am already coordinating with @HassanAkbar on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@suleman-uzair can you help check with @HassanAkbar on how the best behavior in inheriting the mappings?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ronaldtse, I just attempted to implement inheritance but encountered issues with circular dependencies.

class BaseAttributes < Lutaml::Model::Serialize; end
# Above class initialization will trigger the inherited method.
class Mrow < BaseAttributes; end
class Mstyle < BaseAttributes; end

# below class openning will not trigger the inherited method.
class BaseAttribtues < Lutaml::Model::Serialize
  attribute :mrow_value, Mrow
  attribute :mstyle_value, Mstyle

  xml do 
    map_element :mrow_value, to: :mrow_value
    map_element :mstyle_value, to: :mstyle_value
  end
end

class MathML < BaseAttributes
  xml do 
    root “math”, mixed: true
  end
end

class Mrow < BaseAttributes
  xml do 
    root “mrow”, mixed: true
  end
end

class Mstyle < BaseAttributes
  xml do 
    root “mstyle”, mixed: true
  end
end
<math>
  <mrow>
    <mstyle>
      <mrow>
        <!-- further tags and content -->
      </mrow>
    </mstyle>
  </mrow>
</math>

The code above demonstrates an issue with inheritance.
In the XML example, the mstyle and mrow tags depend on each other, creating a circular dependency.
This circular dependency is causing problems with the inheritance structure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is a general lutaml-model issue. I suppose we have to delay the evaluation of those "classes" we pass to attribute?

@ronaldtse
Copy link
Contributor

@Novtopro FYI the MathML gem is being worked on here using lutaml-model.

Related to: metanorma/niso-jats#19

@ronaldtse
Copy link
Contributor

@suleman-uzair do you have an ETA on when this will be done? Thanks.

@suleman-uzair
Copy link
Member Author

@suleman-uzair do you have an ETA on when this will be done? Thanks.

@ronaldtse, As you already know, I’m working on it parallel to plurimath#304(It’s almost finalized, just fixing some failing specs).

I’m hoping to finalize this PR by next Wednesday.

mml.gemspec Outdated
spec.add_runtime_dependency "lutaml-model", "~> 0.3"
spec.add_runtime_dependency "ox"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we defer the decision of what XML gem to use to lutaml-model?

Copy link
Member Author

@suleman-uzair suleman-uzair Nov 6, 2024

Choose a reason for hiding this comment

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

Can we defer the decision of what XML gem to use to lutaml-model?

@ronaldtse, I added "Ox" as dependency because the Nokogiri gem doesn’t handle HTML entities other than &, < ,> , " , and '.
I'll move it to Gemfile, create an issue for this in Lutaml-Model, and add a note mentioning this issue in Mml’s documentation.

mml.gemspec Outdated
spec.add_runtime_dependency "lutaml-model", "~> 0.3"
spec.add_runtime_dependency "ox"

spec.add_development_dependency "equivalent-xml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this to Gemfile.

lib/mml/mrow.rb Outdated Show resolved Hide resolved
@suleman-uzair suleman-uzair changed the title [WIP] MathML parsing using LutaML-Model MathML parsing using LutaML-Model Nov 12, 2024
@suleman-uzair suleman-uzair marked this pull request as ready for review November 12, 2024 11:24
removed Parser namespace, generated parsing files

Updated namespaces, configuration, and parsing attributes

Updated attributes, and parsing structure

Added model class from configuration for all files and mn, ms, mi, etc classes in math, mstyle, and mrow files

removed namespace for all the tags, updated parsing for Munderover, Msubsup, and others

updated namespace for all tags/classes and linked table, tr, and td with math and other main tags

Added msqrt files

Added msqrt, fenced, and munder classes support

Linked Mroot with root/collection classes

is_mrow for mrow specific check for Plurimath::Math::Formula class

Removed namespace prefix from all files, added msgroup support

Added msgroup, msline, mstack, semantics, and others

Added ms and mrow contents parsing

Added/updated merror, mlongdiv and other tags

Added mpadded and menclose support and updated configuration

Added support for Mfraction, Mphantom, and Mmultiscripts tags

Updated attributes for ms tag

Added mglyph tag support

added lutaml-model version and updated mi and mfenced files

disabled top level documentation

removed namepspace from all files and changed lutaml-model to git branch for testing

test: removed lutaml-model deleted branch

WIP: added specs and relevant changes

Added/fixed spec

fix: rubocop offenses

Added zeitwerk and fixed rubocop offenses

fixed rubocop offenses

removed table_cell_expression file and content

chore: removed unused line from table file

chore: moved development gem from gemspec to gemfile
@ronaldtse ronaldtse merged commit 64cd315 into main Nov 18, 2024
13 of 14 checks passed
@ronaldtse
Copy link
Contributor

Thank you @suleman-uzair !

@ronaldtse ronaldtse deleted the features/mathml_parsing branch November 18, 2024 09: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.

Implement mml gem for MathML parsing using lutaml-model
2 participants