Skip to content

Commit

Permalink
Fixes, refactoring, clean up dead code (#297)
Browse files Browse the repository at this point in the history
* Remove unnecessary None check

* Collapse two union types to one

* Don't create language objects for empty strings

* Strip whitespace from user input file path

* Add test for empty value language string
  • Loading branch information
mmwinther authored Apr 17, 2024
1 parent 499164d commit 9c2bb85
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 76 deletions.
4 changes: 0 additions & 4 deletions src/datadoc/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@
from datadoc.backend.statistic_subject_mapping import StatisticSubjectMapping
from datadoc.enums import SupportedLanguages
from datadoc.frontend.callbacks.register_callbacks import register_callbacks
from datadoc.frontend.components.alerts import dataset_validation_error
from datadoc.frontend.components.alerts import naming_convention_warning
from datadoc.frontend.components.alerts import opened_dataset_error
from datadoc.frontend.components.alerts import opened_dataset_success
from datadoc.frontend.components.alerts import saved_metadata_success
from datadoc.frontend.components.alerts import variables_validation_error
from datadoc.frontend.components.control_bars import build_controls_bar
from datadoc.frontend.components.control_bars import build_language_dropdown
from datadoc.frontend.components.control_bars import header
Expand Down Expand Up @@ -61,8 +59,6 @@ def build_app(app: type[Dash]) -> Dash:
[
progress_bar,
build_controls_bar(),
variables_validation_error,
dataset_validation_error,
opened_dataset_error,
saved_metadata_success,
opened_dataset_success,
Expand Down
26 changes: 12 additions & 14 deletions src/datadoc/frontend/callbacks/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@
from datadoc.frontend.callbacks.utils import parse_and_validate_dates
from datadoc.frontend.callbacks.utils import update_global_language_state
from datadoc.frontend.fields.display_dataset import DISPLAYED_DATASET_METADATA
from datadoc.frontend.fields.display_dataset import DISPLAYED_DROPDOWN_DATASET_METADATA
from datadoc.frontend.fields.display_dataset import MULTIPLE_LANGUAGE_DATASET_METADATA
from datadoc.frontend.fields.display_dataset import DROPDOWN_DATASET_METADATA
from datadoc.frontend.fields.display_dataset import (
DROPDOWN_DATASET_METADATA_IDENTIFIERS,
)
from datadoc.frontend.fields.display_dataset import (
MULTIPLE_LANGUAGE_DATASET_IDENTIFIERS,
)
from datadoc.frontend.fields.display_dataset import DatasetIdentifiers
from datadoc.utils import METADATA_DOCUMENT_FILE_SUFFIX

Expand All @@ -40,14 +45,14 @@ def open_file(file_path: str | None = None) -> DataDocMetadata:
logger.info("Opening existing metadata document %s", file_path)
return DataDocMetadata(
state.statistic_subject_mapping,
metadata_document_path=file_path,
metadata_document_path=file_path.strip(),
)

dataset = file_path or get_dataset_path()
logger.info("Opening dataset %s", dataset)
return DataDocMetadata(
state.statistic_subject_mapping,
dataset_path=str(dataset) if dataset else None,
dataset_path=str(dataset).strip() if dataset else None,
)


Expand Down Expand Up @@ -113,7 +118,7 @@ def process_special_cases(
updated_value = process_keyword(value)
elif metadata_identifier == DatasetIdentifiers.VERSION.value:
updated_value = str(value)
elif metadata_identifier in MULTIPLE_LANGUAGE_DATASET_METADATA and isinstance(
elif metadata_identifier in MULTIPLE_LANGUAGE_DATASET_IDENTIFIERS and isinstance(
value,
str,
):
Expand All @@ -124,12 +129,7 @@ def process_special_cases(
metadata_identifier,
language,
)
elif (
metadata_identifier == DatasetIdentifiers.DATASET_STATUS.value
and value == ""
or metadata_identifier == DatasetIdentifiers.DATASET_STATE.value
and value == ""
):
elif metadata_identifier in DROPDOWN_DATASET_METADATA_IDENTIFIERS and value == "":
updated_value = None
else:
updated_value = value
Expand All @@ -150,8 +150,6 @@ def accept_dataset_metadata_input(
metadata_identifier,
)
try:
if language is None:
value = process_special_cases(value, metadata_identifier)
value = process_special_cases(value, metadata_identifier, language)
# Update the value in the model
setattr(
Expand Down Expand Up @@ -198,7 +196,7 @@ def change_language_dataset_metadata(
"""
update_global_language_state(language)
return (
*(e.options_getter(language) for e in DISPLAYED_DROPDOWN_DATASET_METADATA),
*(e.options_getter(language) for e in DROPDOWN_DATASET_METADATA),
update_dataset_metadata_language(),
)

Expand Down
13 changes: 8 additions & 5 deletions src/datadoc/frontend/callbacks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,8 @@ def find_existing_language_string(
value: str,
metadata_identifier: str,
language: str,
) -> model.LanguageStringType:
) -> model.LanguageStringType | None:
"""Get or create a LanguageStrings object and return it."""
# In this case we need to set the string to the correct language code

language_strings = getattr(metadata_model_object, metadata_identifier)

if language_strings is not None:
Expand All @@ -122,13 +120,15 @@ def find_existing_language_string(
language,
value,
)
else:
elif value != "":
_add_language_string_item(
language_strings,
language,
value,
)
else:
else:
return None
elif value != "":
language_strings = model.LanguageStringType(
root=[
model.LanguageStringTypeItem(
Expand All @@ -137,6 +137,9 @@ def find_existing_language_string(
),
],
)
else:
# Don't create an item if the value is empty
return None
return language_strings


Expand Down
14 changes: 0 additions & 14 deletions src/datadoc/frontend/components/alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,6 @@
from datadoc.frontend.components.builders import AlertTypes
from datadoc.frontend.components.builders import build_ssb_alert

dataset_validation_error = build_ssb_alert(
AlertTypes.ERROR,
"dataset-validation-error",
"Validering feilet",
"dataset-validation-explanation",
)

variables_validation_error = build_ssb_alert(
AlertTypes.ERROR,
"variables-validation-error",
"Validering feilet",
"variables-validation-explanation",
)

opened_dataset_error = build_ssb_alert(
AlertTypes.ERROR,
"opened-dataset-error",
Expand Down
14 changes: 3 additions & 11 deletions src/datadoc/frontend/components/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

from datadoc.frontend.fields.display_base import DATASET_METADATA_INPUT
from datadoc.frontend.fields.display_base import VARIABLES_METADATA_INPUT
from datadoc.frontend.fields.display_base import DatasetFieldTypes
from datadoc.frontend.fields.display_base import VariablesFieldTypes
from datadoc.frontend.fields.display_base import FieldTypes

if TYPE_CHECKING:
from datadoc_model import model
Expand Down Expand Up @@ -104,7 +103,7 @@ def build_ssb_alert( # noqa: PLR0913 not immediately obvious how to improve thi


def build_input_field_section(
metadata_fields: list[VariablesFieldTypes],
metadata_fields: list[FieldTypes],
variable: model.Variable,
language: str,
) -> dbc.Form:
Expand Down Expand Up @@ -155,13 +154,6 @@ def build_ssb_accordion(
header=header,
id=key,
children=[
html.Section(
id={
"type": "variable-input-alerts",
"variable_short_name": variable_short_name,
},
className="alert-section",
),
html.Section(
id={
"type": "variable-inputs",
Expand All @@ -176,7 +168,7 @@ def build_ssb_accordion(

def build_dataset_edit_section(
title: str,
metadata_inputs: list[DatasetFieldTypes],
metadata_inputs: list[FieldTypes],
language: str,
dataset: model.Dataset,
key: dict,
Expand Down
10 changes: 1 addition & 9 deletions src/datadoc/frontend/fields/display_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,18 +378,10 @@ def render(
)


VariablesFieldTypes = (
FieldTypes = (
MetadataInputField
| MetadataDropdownField
| MetadataCheckboxField
| MetadataPeriodField
| MetadataMultiLanguageField
)

DatasetFieldTypes = (
MetadataInputField
| MetadataDropdownField
| MetadataPeriodField
| MetadataCheckboxField
| MetadataMultiLanguageField
)
13 changes: 8 additions & 5 deletions src/datadoc/frontend/fields/display_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from datadoc import state
from datadoc.frontend.fields.display_base import DATASET_METADATA_DATE_INPUT
from datadoc.frontend.fields.display_base import DATASET_METADATA_MULTILANGUAGE_INPUT
from datadoc.frontend.fields.display_base import DatasetFieldTypes
from datadoc.frontend.fields.display_base import FieldTypes
from datadoc.frontend.fields.display_base import MetadataCheckboxField
from datadoc.frontend.fields.display_base import MetadataDropdownField
from datadoc.frontend.fields.display_base import MetadataInputField
Expand Down Expand Up @@ -108,7 +108,7 @@ class DatasetIdentifiers(str, Enum):

DISPLAY_DATASET: dict[
DatasetIdentifiers,
DatasetFieldTypes,
FieldTypes,
] = {
DatasetIdentifiers.SHORT_NAME: MetadataInputField(
identifier=DatasetIdentifiers.SHORT_NAME.value,
Expand Down Expand Up @@ -326,7 +326,7 @@ class DatasetIdentifiers(str, Enum):
if v.multiple_language_support:
v.value_getter = get_multi_language_metadata

MULTIPLE_LANGUAGE_DATASET_METADATA = [
MULTIPLE_LANGUAGE_DATASET_IDENTIFIERS = [
m.identifier for m in DISPLAY_DATASET.values() if m.multiple_language_support
]

Expand All @@ -342,15 +342,18 @@ class DatasetIdentifiers(str, Enum):


# The order of this list MUST match the order of display components, as defined in DatasetTab.py
DISPLAYED_DATASET_METADATA: list[DatasetFieldTypes] = (
DISPLAYED_DATASET_METADATA: list[FieldTypes] = (
OBLIGATORY_EDITABLE_DATASET_METADATA
+ OPTIONAL_DATASET_METADATA
+ NON_EDITABLE_DATASET_METADATA
)

DISPLAYED_DROPDOWN_DATASET_METADATA: list[MetadataDropdownField] = [
DROPDOWN_DATASET_METADATA: list[MetadataDropdownField] = [
m for m in DISPLAYED_DATASET_METADATA if isinstance(m, MetadataDropdownField)
]
DROPDOWN_DATASET_METADATA_IDENTIFIERS: list[str] = [
m.identifier for m in DROPDOWN_DATASET_METADATA
]

OBLIGATORY_DATASET_METADATA_IDENTIFIERS: list[str] = [
m.identifier for m in DISPLAY_DATASET.values() if m.obligatory and m.editable
Expand Down
4 changes: 2 additions & 2 deletions src/datadoc/frontend/fields/display_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
from datadoc import enums
from datadoc.frontend.fields.display_base import VARIABLES_METADATA_DATE_INPUT
from datadoc.frontend.fields.display_base import VARIABLES_METADATA_MULTILANGUAGE_INPUT
from datadoc.frontend.fields.display_base import FieldTypes
from datadoc.frontend.fields.display_base import MetadataCheckboxField
from datadoc.frontend.fields.display_base import MetadataDropdownField
from datadoc.frontend.fields.display_base import MetadataInputField
from datadoc.frontend.fields.display_base import MetadataMultiLanguageField
from datadoc.frontend.fields.display_base import MetadataPeriodField
from datadoc.frontend.fields.display_base import VariablesFieldTypes
from datadoc.frontend.fields.display_base import get_enum_options_for_language
from datadoc.frontend.fields.display_base import get_multi_language_metadata

Expand Down Expand Up @@ -44,7 +44,7 @@ class VariableIdentifiers(str, Enum):

DISPLAY_VARIABLES: dict[
VariableIdentifiers,
VariablesFieldTypes,
FieldTypes,
] = {
VariableIdentifiers.SHORT_NAME: MetadataInputField(
identifier=VariableIdentifiers.SHORT_NAME.value,
Expand Down
18 changes: 15 additions & 3 deletions tests/frontend/callbacks/test_callbacks_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,29 @@

def test_find_existing_language_string_no_existing_strings(bokmål_name: str):
dataset_metadata = model.Dataset()
language_strings = find_existing_language_string(
assert find_existing_language_string(
dataset_metadata,
bokmål_name,
"name",
"nb",
)
assert language_strings == model.LanguageStringType(
) == model.LanguageStringType(
[model.LanguageStringTypeItem(languageCode="nb", languageText=bokmål_name)],
)


def test_find_existing_language_string_no_existing_strings_empty_value():
dataset_metadata = model.Dataset()
assert (
find_existing_language_string(
dataset_metadata,
"",
"name",
"nb",
)
is None
)


def test_find_existing_language_string_pre_existing_strings(
english_name: str,
bokmål_name: str,
Expand Down
17 changes: 13 additions & 4 deletions tests/frontend/callbacks/test_dataset_callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
from datadoc.frontend.callbacks.dataset import open_dataset_handling
from datadoc.frontend.callbacks.dataset import process_special_cases
from datadoc.frontend.callbacks.dataset import update_dataset_metadata_language
from datadoc.frontend.fields.display_dataset import MULTIPLE_LANGUAGE_DATASET_METADATA
from datadoc.frontend.fields.display_dataset import (
MULTIPLE_LANGUAGE_DATASET_IDENTIFIERS,
)
from datadoc.frontend.fields.display_dataset import DatasetIdentifiers
from tests.utils import TEST_PARQUET_FILEPATH

Expand Down Expand Up @@ -312,17 +314,24 @@ def test_change_language_dataset_metadata_options_enums(
assert {d["id"] for d in test_without_empty_option} == {e.name for e in enum_for_options} # type: ignore [attr-defined]


@pytest.mark.parametrize(
"path",
[
"valid/path/to/person_data_v1.parquet",
" path/with/extra/whitespace/person_data_v1.parquet ",
],
)
@patch(f"{DATASET_CALLBACKS_MODULE}.open_file")
def test_open_dataset_handling_normal(
open_file_mock: Mock, # noqa: ARG001
n_clicks_1: int,
file_path: str,
path: str,
):
state.current_metadata_language = SupportedLanguages.ENGLISH

opened, show_error, naming_standard, error_msg, language = open_dataset_handling(
n_clicks_1,
file_path,
path,
)
assert opened
assert not show_error
Expand Down Expand Up @@ -422,7 +431,7 @@ def test_process_special_cases_language_string(
language = "en"
value = "Test language string"
identifier = random.choice( # noqa: S311
MULTIPLE_LANGUAGE_DATASET_METADATA,
MULTIPLE_LANGUAGE_DATASET_IDENTIFIERS,
)
expected = model.LanguageStringType(
[
Expand Down
Loading

0 comments on commit 9c2bb85

Please sign in to comment.