Skip to content
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.

Sign up for GitHub

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

BF: move check for metalad before import of meta_extract #257

Merged
merged 2 commits into from
Jan 17, 2024
Merged

BF: move check for metalad before import of meta_extract #257

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3803bf7
BF: move check for metalad before import of meta_extract
yarikoptic Jan 17, 2024
be5f9e1
isort the test file
yarikoptic Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions datalad_container/extractors/tests/test_metalad_container.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,31 @@
import os.path as op
import pytest
import subprocess
import sys
from pathlib import Path
from shutil import which
from unittest.mock import patch

import pytest
from datalad.support.external_versions import (
UnknownVersion,
external_versions,
)
# Early detection before we try to import meta_extract
from datalad.tests.utils_pytest import SkipTest

if not external_versions["datalad_metalad"]:
raise SkipTest("skipping metalad tests")

Check warning on line 17 in datalad_container/extractors/tests/test_metalad_container.py

View check run for this annotation

Codecov / codecov/patch

datalad_container/extractors/tests/test_metalad_container.py#L17 

Added line #L17 was not covered by tests

from datalad.api import (
clone,
Dataset,
clone,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a convention to import classes first? I've always just done strictly alphabetical

Copy link
Member Author

Choose a reason for hiding this comment

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

"not sure". Please find an answer at https://pycqa.github.io/isort/docs/configuration/custom_sections_and_ordering.html somewhere or may be @jwodder could clarify?

Copy link
Member

Choose a reason for hiding this comment

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

isort does sort by case

https://pycqa.github.io/isort/docs/configuration/options.html

Order imports by type, which is determined by case, in addition to alphabetically.

NOTE: type here refers to the implied type from the import name capitalization. isort does not do type introspection for the imports. These "types" are simply: CONSTANT_VARIABLE, CamelCaseClass, variable_or_function. If your project follows PEP8 or a related coding standard and has many imports this is a good default, otherwise you likely will want to turn it off. From the CLI the --dont-order-by-type option will turn this off.

Type: Bool
Default: True
Config default: true
Python & Config File Name: order_by_type
CLI Flags:

--ot

--order-by-type

meta_extract,
)
from datalad.cmd import (
StdOutCapture,
WitlessRunner,
)
from datalad.support.exceptions import CommandError
from datalad.support.external_versions import external_versions, UnknownVersion
from datalad.tests.utils_pytest import (
SkipTest,
assert_in,
Expand All @@ -27,9 +36,6 @@
with_tree,
)

if not external_versions["datalad_metalad"]:
raise SkipTest("skipping metalad tests")

from datalad_container.utils import get_container_command

try:
Expand All @@ -38,7 +44,9 @@
raise SkipTest("skipping singularity/apptainer tests")

# Must come after skiptest or imports will not work
from datalad_container.extractors.metalad_container import MetaladContainerInspect
from datalad_container.extractors.metalad_container import (
MetaladContainerInspect,
)


@with_tempfile
Expand Down
Loading