Addition of subyt to sema #67
Conversation
| @@ -84,7 +84,7 @@ test-coverage-with-graphdb: ## runs the standard test-suite for all available i | |||
| check: ## performs linting on the python code | |||
| @poetry run black --check --diff . | |||
| @poetry run isort --check --diff . | |||
| @poetry run flake8 . --exclude ${FLAKE8_EXCLUDE} | |||
| @poetry run flake8 . --exclude ${FLAKE8_EXCLUDE} --ignore=E203,W503 | |||
There was a problem hiding this comment.
W503 line break before binary operator -> doesn't play well with max line length
E203 local black formattor kept undoing changes with format on save -> psf/black#315
Tests coverage table for 05cb08b commit.
|
pyproject.toml
Outdated
| @@ -16,6 +16,10 @@ python-dateutil = "^2.9.0.post0" | |||
| setuptools = "*" | |||
| uritemplate = "^4.1.1" | |||
| urllib3 = "^2.2.2" | |||
| requests = "^2.32.3" | |||
| typeguard = "^4.3.0" | |||
| xmlasdict = "^0.2.2" | |||
There was a problem hiding this comment.
xmlasdict dependency is also not required for subyt use - only when you want to deal with xml inputs (which we do, but only inside the tests?)
caveat -- documentation will need to make clear that such usage declares its own dependency on xmlasdict
makes me think (new issue?) we could also register a fake source class that makes that statement explicitely when people try inputtinh files that do require the XMLSource
There was a problem hiding this comment.
note: currently there is a log.warn already about the xml support being not available when the xmlasdict dependency is not met -- as suggested above, we might extend that policy with a more upfront message / fail when XML processing is actually attempted later
There was a problem hiding this comment.
Further handling done by #68
| log.info(f"Logging enabled according to config in {args.logconf}") | ||
|
|
||
|
|
||
| def main(): |
There was a problem hiding this comment.
can main() be made to use Sybut ?
if not now -- in a todo-issue for later?
| """Source context cleanup""" | ||
|
|
||
|
|
||
| # TODO make a pandas source in sources.py |
There was a problem hiding this comment.
remove this comment, rather introduce an issue at gh for this?
| args = get_arg_parser().parse_args() | ||
|
|
||
| enable_logging(args) | ||
| service = make_service(args) |
There was a problem hiding this comment.
precursor of the new style "service" interface introduced by sembench
-- maybe we should consider supporting that way of calling the core process with string-arguments only already?
sema/subyt/j2/generator.py
Outdated
| :param templates_folder: Location of the templates defaults to "." | ||
| """ | ||
| self._templates_folder = templates_folder | ||
| ttl_filter = {"ttl": Filters.all()["xsd"]} |
There was a problem hiding this comment.
do we still want this?
move to sema looks like a good time to get rid off the older ttl syntax?
There was a problem hiding this comment.
I have no idea what this does so I'm leaving it here for now
There was a problem hiding this comment.
this keeps | ttl working as it looks up the xsd filter and registers it again as ttl too
There was a problem hiding this comment.
- missing some opportunities to do cleanup of some historic stuff
- also introduces new stuff that should be reconsidered imho (mainly a smart selection of when to load_log_config)
- introducing unwanted dependencies too
- unsure about the increased leniency on the discovery test
surely would like to see some fix on the dependency management, other things could move to follow up issues if need be
handles the following issues: