Update UnitsML for latest MathML, Plurimath, and UnitsDB integration changes#61
Update UnitsML for latest MathML, Plurimath, and UnitsDB integration changes#61
Conversation
# Conflicts: # Gemfile # lib/unitsml/formula.rb # lib/unitsml/unit.rb
There was a problem hiding this comment.
Pull request overview
This draft PR updates UnitsML’s integration points with the latest MathML (MML v4), Plurimath parsing, and unitsdb-ruby’s context/register loading model, including Opal-compatible database loading behavior.
Changes:
- Introduce a shared
MathmlHelperand update MathML generation to use MML v4 context/register APIs. - Replace the prior Unitsdb/Lutaml global register wiring with a new
Unitsml::Configurationcontext + model substitution registration flow. - Add Opal-specific Unitsdb database loading support and corresponding specs/error type.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/unitsml_spec.rb | Adds a basic parse regression spec for a prefixed unit (“mm”). |
| spec/unitsml/unitsdb/database_spec.rb | Adds specs for UnitsML’s Unitsdb database loader behavior (ruby vs opal). |
| lib/unitsml/utility.rb | Updates XML generation to pass lutaml_register, refactors prefix handling/helpers, and tweaks unit/dimension/quantity XML serialization calls. |
| lib/unitsml/unitsdb/units.rb | Removes manual wrapping setter and registers the model for context substitution. |
| lib/unitsml/unitsdb/unit.rb | Removes manual wrapping setter and registers the model for context substitution. |
| lib/unitsml/unitsdb/si_derived_base.rb | Removes the wrapper class (no longer needed with new substitution flow). |
| lib/unitsml/unitsdb/quantities.rb | Registers the model for context substitution. |
| lib/unitsml/unitsdb/prefixes.rb | Registers the model for context substitution. |
| lib/unitsml/unitsdb/prefix_reference.rb | Registers the model for context substitution. |
| lib/unitsml/unitsdb/dimensions.rb | Registers the model for context substitution. |
| lib/unitsml/unitsdb/dimension_details.rb | Renames/aligns wrapper class and registers the model for context substitution. |
| lib/unitsml/unitsdb/dimension.rb | Simplifies dimension handling and registers the model for context substitution. |
| lib/unitsml/unitsdb/database.rb | Adds UnitsML database wrapper with Opal payload loading and model registration. |
| lib/unitsml/unitsdb.rb | Reworks Unitsdb loading/caching and introduces filesystem-path probing/fallbacks. |
| lib/unitsml/unit.rb | Updates Unit MathML generation to use helper/context-aware constructors. |
| lib/unitsml/prefix.rb | Updates Prefix MathML generation to use helper/context-aware constructors. |
| lib/unitsml/number.rb | Updates Number MathML generation to use helper/context-aware constructors. |
| lib/unitsml/mathml_helper.rb | Adds helper module to centralize MML v4 context/register element creation/parsing. |
| lib/unitsml/formula.rb | Updates MathML serialization to use helper/context and compacts MathML before Plurimath parse. |
| lib/unitsml/fenced.rb | Updates fenced-parenthesis MathML generation to use helper/context-aware constructors. |
| lib/unitsml/extender.rb | Updates extender/operator MathML generation to use helper/context-aware constructors. |
| lib/unitsml/errors/opal_payload_not_bundled_error.rb | Adds a dedicated error type for missing Opal-bundled Unitsdb payload. |
| lib/unitsml/errors.rb | Autoloads the new Opal payload error. |
| lib/unitsml/dimension.rb | Updates Dimension MathML/XML generation to use helper/context-aware constructors. |
| lib/unitsml/configuration.rb | Introduces UnitsML-specific Unitsdb context creation + model substitution registry. |
| lib/unitsml.rb | Removes old global register setup and autoloads the new configuration/helper modules. |
| docs/README.adoc | Updates documentation to reflect unitsdb-ruby as the runtime data source and documents dev setup/fallbacks. |
| Gemfile | Pins integration branches for lutaml-model/mml/plurimath/unitsdb-ruby for coordinated changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ::Unitsdb::Database.from_db(database_path) | ||
| rescue ::Unitsdb::Errors::DatabaseNotFoundError, | ||
| ::Unitsdb::Errors::DatabaseFileNotFoundError | ||
| ::Unitsdb::Database.from_db(database_path) |
There was a problem hiding this comment.
load_database falls back to ::Unitsdb::Database.from_db(database_path) without passing the configured context. That bypasses the UnitsML context/substitution flow and can return base Unitsdb models instead of the registered Unitsml wrappers. Consider calling Unitsml::Unitsdb::Database.from_db(database_path, context: Configuration.context.id) (or passing context: to ::Unitsdb::Database.from_db) in both the normal and rescue paths. Also, the rescue block currently retries the exact same call, which is unlikely to change the outcome; either remove it or make it attempt an actual fallback path/strategy.
| ::Unitsdb::Database.from_db(database_path) | |
| rescue ::Unitsdb::Errors::DatabaseNotFoundError, | |
| ::Unitsdb::Errors::DatabaseFileNotFoundError | |
| ::Unitsdb::Database.from_db(database_path) | |
| Database.from_db(database_path, context: Configuration.context.id) |
| def to_mathml(options = {}) | ||
| if root | ||
| options = update_options(options) | ||
| nullify_mml_models if plurimath_available? | ||
| math = ::Mml::V4::Math.new(display: "block") | ||
| math = mml_v4_new(:math, display: "block") | ||
| math.ordered = true | ||
| math.element_order ||= [] | ||
| value.each do |instance| | ||
| process_value(math, instance.to_mathml(options)) | ||
| end | ||
| generated_math = math.to_xml.gsub(%r{&(.*?)(?=</)}, '&\1') | ||
| reset_mml_models if plurimath_available? | ||
| generated_math = math.to_xml(register: mml_v4_context.id) | ||
| .gsub(%r{&(.*?)(?=</)}, '&\1') | ||
|
|
There was a problem hiding this comment.
to_mathml no longer calls nullify_mml_models / reset_mml_models, but the private methods remain in this file and are now unused. Consider removing them to reduce maintenance surface, or reintroduce the call site if they are still required for Plurimath/MML interoperability.
|
|
||
| The standard Ruby runtime path goes through `::Unitsdb.database`. When | ||
| `unitsdb-ruby` cannot load its bundled `data/` directory, UnitsML falls back to | ||
| the packaged YAML files under `vendor/unitsdb`. |
There was a problem hiding this comment.
The fallback description is ambiguous: the code resolves vendor/unitsdb relative to the unitsdb-ruby gem path (not this repository). Consider clarifying the docs to explicitly say the fallback is to the unitsdb-ruby gem’s packaged vendor/unitsdb directory, so readers don’t assume unitsml-ruby/vendor/unitsdb is required at runtime.
| the packaged YAML files under `vendor/unitsdb`. | |
| the packaged YAML files in the `unitsdb-ruby` gem's `vendor/unitsdb` | |
| directory, rather than a `vendor/unitsdb` directory in this repository. |
| def load_database | ||
| if RUBY_ENGINE == "opal" | ||
| return Database.from_db(nil, context: Configuration.context.id) | ||
| end | ||
|
|
||
| if ::Unitsdb.respond_to?(:database) | ||
| return ::Unitsdb.database(context: Configuration.context.id) | ||
| end | ||
|
|
||
| ::Unitsdb::Database.from_db(database_path) | ||
| rescue ::Unitsdb::Errors::DatabaseNotFoundError, | ||
| ::Unitsdb::Errors::DatabaseFileNotFoundError | ||
| ::Unitsdb::Database.from_db(database_path) | ||
| end |
There was a problem hiding this comment.
The new database_path / fallback loader logic (including the ::Unitsdb.database(context: ...) branch and the ::Unitsdb::Database.from_db fallback) is not covered by specs. Adding tests for the non-Opal paths would help catch regressions across unitsdb-ruby versions and packaging layouts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "#{prefix.to_mathml(options)}#{value.value}") | ||
| value = mml_v4_with_content( | ||
| value, | ||
| "#{prefix.to_mathml(**options, parent: value)}#{value.value}", |
There was a problem hiding this comment.
prefix.to_mathml(**options, parent: value) passes keyword arguments to Prefix#to_mathml, but Unitsml::Prefix#to_mathml only accepts a single positional options argument. This will raise ArgumentError at runtime when a prefixed unit is converted to MathML. Pass a hash instead (e.g., merge parent into the options hash) or update to_mathml signatures consistently to accept keywords.
| "#{prefix.to_mathml(**options, parent: value)}#{value.value}", | |
| "#{prefix.to_mathml(options.merge(parent: value))}#{value.value}", |
| end | ||
| end | ||
| end | ||
|
|
||
| Configuration.register_model(PrefixReference, id: :prefix_reference) |
There was a problem hiding this comment.
PrefixReference#prefix uses ::Unitsdb.database, which bypasses Unitsml::Unitsdb.database (and its context-aware / Opal-aware loading). In Opal runtimes this can trigger filesystem-based loading or a different database instance than the one UnitsML configured. Prefer looking up prefixes via Unitsml::Unitsdb.database (or ::Unitsdb.database(context: Unitsml::Configuration.context.id)) so the same context and payload are used.
| resolved_prefix = resolved_prefix(prefix) | ||
| return unless resolved_prefix | ||
|
|
||
| resolved_prefix.symbols&.first&.ascii |
There was a problem hiding this comment.
In prefix_symbolid, the local variable name resolved_prefix shadows the helper method resolved_prefix, which makes the code harder to read and easy to misinterpret when debugging. Rename the local variable (e.g., prefix_record) to avoid the shadowing.
| resolved_prefix = resolved_prefix(prefix) | |
| return unless resolved_prefix | |
| resolved_prefix.symbols&.first&.ascii | |
| prefix_record = resolved_prefix(prefix) | |
| return unless prefix_record | |
| prefix_record.symbols&.first&.ascii |
This draft PR updates UnitsML to align with the latest MML, Plurimath, and UnitsDB changes.
[Work-in-progress]: I am still working on Opal / Plurimath-js compatibility, especially around UnitsDB loading without filesystem access in browser-style runtimes.