Skip to content

restore dataclass_extensions. See #345 #384

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

Merged
merged 17 commits into from
Apr 4, 2025

Conversation

cmungall
Copy link
Member

@cmungall cmungall commented Mar 30, 2025

upstream_repo: linkml/linkml
upstream_branch: pr-2358

This was prematurely removed in #345. A lot of existing code depends on this

Note the changes from #345 temporarily made their way into a 1.9.0 release, but this was promptly yanked

Copy link

codecov bot commented Mar 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.79%. Comparing base (a1db174) to head (8f4ede4).
Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
+ Coverage   63.76%   63.79%   +0.02%     
==========================================
  Files          62       63       +1     
  Lines        8945     8951       +6     
  Branches     2587     2587              
==========================================
+ Hits         5704     5710       +6     
  Misses       2634     2634              
  Partials      607      607              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dalito
Copy link
Member

dalito commented Mar 30, 2025

#345 also removed the test for this code which was in the file tests/test_issues/test_issue_718a.py

@cmungall
Copy link
Member Author

cmungall commented Mar 30, 2025

This looks like it's failing some of the downstream linkml tests.

I looked at the first one, which dates back to:
biolink/biolinkml#326

The test is

FAILED tests/test_issues/test_issue_316.py::test_alt_description_2 - TypeError: __init__() got an unexpected keyword argument 'text'

def test_alt_description_2(input_path):
    with pytest.raises(ValueError, match="description must be supplied"):
        YAMLGenerator(input_path("issue_326a.yaml")).serialize(validateonly=True)

This is a little rigid. An exception is still raised, but with the changes in #345, the reason/exception type/message changes (since it is now caught at init time)

Same with

FAILED tests/test_utils/test_load_raw_schema.py::test_representation_errors - TypeError: __init__() got an unexpected keyword argument 'schema1'

def test_representation_errors(input_path):
    """Test malformed schema elements"""
    fn = input_path("typeerror1.yaml")
    with pytest.raises(ValueError):
        SchemaLoader(fn)

and

FAILED tests/test_validator/test_validate.py::test_not_a_valid_schema - TypeError: __init__() got an unexpected keyword argument 'foo'

def test_not_a_valid_schema():
    instance = {}
    schema = {"foo": "bar"}
    with pytest.raises(ValueError, match="Invalid schema"):
        validate(instance, schema)

These tests have been switched to use the regenerated linkml-model code

linkml/linkml#2589

but they have become too rigid in the other direction

What I am surprised about is how this made it through before..

@dalito
Copy link
Member

dalito commented Apr 2, 2025

This seems to fail because dynamic versioning has to be installed differently with poetry 2.x

After pipx install poetry add pipx inject poetry poetry-dynamic-versioning (as in #364)

@sierra-moxon
Copy link
Member

🍏 📗 🟢 💚

@sierra-moxon sierra-moxon self-requested a review April 4, 2025 22:00
Copy link
Member

@sierra-moxon sierra-moxon left a comment

Choose a reason for hiding this comment

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

🥗

@sierra-moxon sierra-moxon merged commit 905789c into main Apr 4, 2025
15 checks passed
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.

3 participants