Remove redundant subClassOf axioms with cardinality restriction preservation #3611
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rvation
- Created script to remove redundant is_a and relationship axioms
- Preserves subClassOf axioms when ANY intersection_of clause has cardinality restrictions
- Examples of preserved restrictions: {cardinality="2"}, {minCardinality="1"}, etc.
- Processed 303 terms, removing 336 redundant axioms
- Created backup file before modifications
- Fixes issue #3548
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@dragon-ai-agent
Contributor
|
#gogoeditdiff |
Contributor
Here's a diff of how these changes impact the classified ontology (on -base file):Ontology comparisonLeft
Right
Ontology importsOntology annotations1st arch mandibular mesenchyme
|
Contributor
Here's a diff of your edit file (unreasoned)Ontology comparisonLeft
Right
Ontology importsOntology annotations1st arch mandibular mesenchyme
|
dosumis
approved these changes
Sep 15, 2025
Contributor
dosumis
left a comment
dosumis
left a comment
There was a problem hiding this comment.
Looks good to me.
The only thing lost is axiom annotation indicating source. Might be possible to preserve on EC expressions if we really want it.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the redundant axiom removal script to preserve subClassOf assertions when intersection_of clauses contain cardinality restrictions. The script now detects cardinality, minCardinality, and maxCardinality patterns and preserves ALL subClassOf axioms for terms that have ANY intersection_of clause with such restrictions.
- Updated script logic to detect cardinality restrictions in intersection_of clauses
- Modified preservation logic to keep all subClassOf axioms when cardinality restrictions are present
- Re-ran the script with improved logic, resulting in 336 redundant axioms removed (down from 375)
| } | ||
|
|
||
| # Create backup | ||
| system("cp '$input_file' '${input_file}${backup_suffix}'"); |
There was a problem hiding this comment.
The system call uses unvalidated user input that could lead to command injection. Use File::Copy's copy() function instead for safer file operations.
| my $intersection_clause = $1; | ||
|
|
||
| # Check for cardinality restrictions | ||
| if ($intersection_clause =~ /\{.*(?:cardinality|minCardinality|maxCardinality)/) { |
There was a problem hiding this comment.
The regex pattern doesn't require the closing brace, which could match incomplete cardinality restrictions. Consider using \{.*(?:cardinality|minCardinality|maxCardinality).*\} to ensure proper bracketing.
Suggested change
| if ($intersection_clause =~ /\{.*(?:cardinality|minCardinality|maxCardinality)/) { | |
| if ($intersection_clause =~ /\{.*(?:cardinality|minCardinality|maxCardinality).*?\}/) { |
| } | ||
|
|
||
| # Parse intersection_of clause | ||
| if ($intersection_clause =~ /^([A-Z_]+:\d+)\s*(?:\{.*\})?\s*(?:!\s*.+)?$/) { |
There was a problem hiding this comment.
[nitpick] The regex patterns for parsing intersection_of clauses are duplicated and complex. Consider extracting them into named variables or a helper function to improve readability and maintainability.
| if ($intersection_clause =~ /^([A-Z_]+:\d+)\s*(?:\{.*\})?\s*(?:!\s*.+)?$/) { | ||
| # intersection_of: CLASS | ||
| $intersection_of_classes{$1} = 1; | ||
| } elsif ($intersection_clause =~ /^(\S+)\s+([A-Z_]+:\d+)\s*(?:\{.*\})?\s*(?:!\s*.+)?$/) { |
There was a problem hiding this comment.
[nitpick] The regex patterns for parsing intersection_of clauses are duplicated and complex. Consider extracting them into named variables or a helper function to improve readability and maintainability.
| # | ||
| # Usage: perl remove-redundant-subclass-axioms.pl input.obo > output.obo | ||
|
|
||
| my $input_file = $ARGV[0] || 'src/ontology/uberon-edit.obo'; |
There was a problem hiding this comment.
Using a hardcoded default file path could lead to unintended file modifications. Consider requiring the input file as a mandatory parameter or validate the default path exists and is writable.
Suggested change
| my $input_file = $ARGV[0] || 'src/ontology/uberon-edit.obo'; | |
| if (!defined $ARGV[0]) { | |
| die "Usage: perl $0 input.obo > output.obo\n"; | |
| } | |
| my $input_file = $ARGV[0]; |
Contributor
|
This PR has not seen any activity in the past month; if nobody comments or reviews it in the next week, the PR creator will be allowed to proceed with merging without explicit approval, should they wish to do so. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Modified the redundant axiom removal script and re-ran it with improved logic to preserve subClassOf assertions when ANY intersection_of clause contains cardinality restrictions.
Key Changes
src/scripts/remove-redundant-subclass-axioms.plto detect cardinality restrictions in intersection_of clausescardinality,minCardinality, andmaxCardinalityrestrictionsExample Preservation
The term
UBERON:0011135(intervertebral cartilage) demonstrates the new behavior:Results
{cardinality="2"}kept their subClassOf axiomsuberon-edit.obo.backup-1757956240Test Plan
Addresses issue #3548
🤖 Generated with Claude Code