new test that fails on digit first or x-digit slot names#755
Open
new test that fails on digit first or x-digit slot names#755
Conversation
turbomam
requested review from
Woolly-at-EBI,
mslarae13,
only1chunts,
ramonawalls,
sformel-usgs and
sujaypatil96
turbomam
changed the title
Contributor
LinkML Linting Resultslinkml, version 1.8.4 linting found the following issues. For more information about the linting rule categories, see the LinkML linter documentation. Summary
Problems per SchemaFile: /home/runner/work/mixs/mixs/src/mixs/schema/mixs.yaml Warning
|
Member
Author
2025-04-21Slots whose names start with 'x' and a digit: ['x16s_recover', 'x16s_recover_software'] |
Member
Author
turbomam
force-pushed
the
754-test-for-slots-whose-names-start-with-a-digit-or-x-followed-by-a-digit
branch
from
79ecc2a to
4ca4295
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a regression test intended to prevent MIxS schema slot names from starting with a digit (or with x followed by a digit), motivated by the ongoing x16s_recover* short-name discussion in #742 / mixs#801.
Changes:
- Introduces a new
unittestthat loadssrc/mixs/schema/mixs.yamlviaSchemaView. - Asserts there are no slots (excluding
MixsCompliantData-domain slots) whose names start with a digit or withx+ digit.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+43
| self.assertListEqual(slot_name_digit_first, [], | ||
| f"Slots whose names start with a digit: {slot_name_digit_first}") | ||
|
|
||
| self.assertListEqual(slot_name_x_digit, [], | ||
| f"Slots whose names start with 'x' and a digit: {slot_name_x_digit}") |
There was a problem hiding this comment.
As written, this test is guaranteed to fail against the current schema because src/mixs/schema/mixs.yaml contains slots like x16s_recover/x16s_recover_software (names starting with x + digit). Since CI runs python -m unittest discover in make test, this will block merging. If the failure is intentional until #742/#801 is addressed, mark the test as an expected failure or skip it with a reference to the issue(s), then flip it back once the schema is updated.
Comment on lines
+15
to
+18
| """Test term name length.""" | ||
|
|
||
| def test_digit_first_term_name(self): | ||
| """Test term name length.""" |
There was a problem hiding this comment.
The docstrings say "term name length", but the test is actually checking for slot names that start with a digit or with x+digit. Updating the class/method docstrings to match the actual assertion would make the intent clearer for future readers.
Suggested change
| """Test term name length.""" | |
| def test_digit_first_term_name(self): | |
| """Test term name length.""" | |
| """Test that slot names do not start with a digit or with 'x' followed by a digit.""" | |
| def test_digit_first_term_name(self): | |
| """Verify that no slot name starts with a digit or with 'x' followed by a digit.""" |
| def test_digit_first_term_name(self): | ||
| """Test term name length.""" | ||
|
|
||
| print("\n") |
There was a problem hiding this comment.
print("\n") adds noise to test output and makes failures harder to scan in CI logs. Consider removing it unless there is a specific debugging need.
Suggested change
| print("\n") |
Comment on lines
+4
to
+5
| import src.mixs.datamodel.mixs as mixs | ||
|
|
There was a problem hiding this comment.
import src.mixs.datamodel.mixs as mixs will raise ModuleNotFoundError during python -m unittest discover because src/ is not a Python package (no src/__init__.py). The import is also unused. Remove it, or import from the installed package namespace (e.g., mixs...) if you intended to validate the generated datamodel is importable.
Suggested change
| import src.mixs.datamodel.mixs as mixs |
Contributor
|
This was referenced Mar 10, 2026
Member
Author
Merge order and requirementsThis PR should merge after #974, which adds taxon-neutral replacements for the Implementation noteCopilot's review flagged several items worth addressing:
Related
|
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
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.
This is expected to fail until the
x16s_recoveris fixed.See