Test on all current Python versions#167
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR expands test coverage across newer Python versions, enhances data type inference for CSV inputs, and introduces slot range normalization in the RDFS importer.
- Update GitHub Actions matrix to include Python 3.11–3.13 and switch to
actions/cache@v3 - Enhance
infer_rangeto distinguishdatevsdatetimeand add_normalize_slot_rangesin the RDFS importer - Migrate Poetry dev dependencies to the new
group.devformat inpyproject.toml
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_importers/test_rdfs_importer.py | Added pytest import and updated expected slot count/checks |
| tests/test_generalizers/test_csv_data_generalizer.py | Added cases for date/datetime inference in infer_range tests |
| schema_automator/importers/rdfs_import_engine.py | Extended type hints, logging warnings on duplicate slots, and added slot range normalization |
| schema_automator/generalizers/csv_data_generalizer.py | Added date-only detection branch before datetime in infer_range |
| pyproject.toml | Converted dev-dependencies section to [tool.poetry.group.dev.dependencies] |
| .github/workflows/check-pull-request.yaml | Expanded matrix to Python 3.11–3.13, removed Windows, and allowed 3.13 to fail |
Comments suppressed due to low confidence (2)
.github/workflows/check-pull-request.yaml:22
- [nitpick] Since
windows-latesthas been removed from the matrix, the existing exclude block for Windows + 3.9 is now redundant; consider removing it to simplify the workflow.
exclude:
pyproject.toml:63
- Verify that your Poetry version supports the new
group.dev.dependenciestable; older versions expecttool.poetry.dev-dependenciesand may break CI.
[tool.poetry.group.dev.dependencies]
| from io import StringIO | ||
| import unittest | ||
| import os | ||
| import pytest |
There was a problem hiding this comment.
The pytest import is never used in this test file; consider removing the unused import.
Suggested change
| import pytest |
| default_prefix: str | None = None, | ||
| model_uri: str | None = None, | ||
| identifier: str | None = None, | ||
| file: Union[str, Path, TextIO], |
There was a problem hiding this comment.
The parameter name file shadows a common built-in; consider renaming it (e.g., source or input_path) for better clarity.
Suggested change
| file: Union[str, Path, TextIO], | |
| input_path: Union[str, Path, TextIO], |
| identifier: str | None = None, | ||
| file: Union[str, Path, TextIO], | ||
| name: Optional[str] = None, | ||
| format: Optional[str] = "turtle", |
There was a problem hiding this comment.
The parameter name format shadows the built-in format function; consider renaming it (e.g., rdf_format) to avoid confusion.
Suggested change
| format: Optional[str] = "turtle", | |
| rdf_format: Optional[str] = "turtle", |
Comment on lines
+648
to
+651
| if all( | ||
| not hasattr(parse(str(v)), 'hour') or | ||
| (parse(str(v)).hour == 0 and parse(str(v)).minute == 0 and parse(str(v)).second == 0) | ||
| for v in nn_vals |
There was a problem hiding this comment.
[nitpick] You call parse(str(v)) twice for each value in the date-only check; consider parsing once per value and reusing the result to eliminate redundant work.
Suggested change
| if all( | |
| not hasattr(parse(str(v)), 'hour') or | |
| (parse(str(v)).hour == 0 and parse(str(v)).minute == 0 and parse(str(v)).second == 0) | |
| for v in nn_vals | |
| parsed_vals = [parse(str(v)) for v in nn_vals] | |
| if all( | |
| not hasattr(pv, 'hour') or | |
| (pv.hour == 0 and pv.minute == 0 and pv.second == 0) | |
| for pv in parsed_vals |
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.
Let's try to get ahead of things and run tests on the other Python versions to make sure we're still passing.