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.

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

Remove warnings in unit tests #509

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dee6b62
Remove warnings in unit tests
danielhollas Sep 3, 2023
106c913
Update precommit action
danielhollas Sep 6, 2023
e29dd56
Ignore remaining warnings, turn warnings into errors
danielhollas Sep 6, 2023
2d7daab
Ignore DeprecationWarning from jupyter_core
danielhollas Sep 6, 2023
b6ee99c
Now?
danielhollas Sep 6, 2023
ff053d4
wtf
danielhollas Sep 6, 2023
0a10d8c
jupyter_client
danielhollas Sep 6, 2023
9ed6fbc
One more?
danielhollas Sep 6, 2023
1f75cb8
ignore selenium deprecationwarnings
danielhollas Sep 7, 2023
e655614
Update selenium
danielhollas Sep 7, 2023
c3f0c5b
Try selenium 4.8
danielhollas Sep 8, 2023
1644d8f
Merge branch 'master' into fix/deprecation-warnings-misc
danielhollas Sep 8, 2023
3bcead9
downgrade selenium back to 4.7
danielhollas Sep 8, 2023
b98285d
Do not error on warnings
danielhollas Sep 9, 2023
144f24f
ignore pytest_selenium deprecation warnings
danielhollas Sep 9, 2023
7896eeb
error on warnings
danielhollas Sep 9, 2023
1167ab6
Merge branch 'master' into fix/deprecation-warnings-misc
danielhollas Sep 25, 2023
339dd61
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 25, 2023
fc58864
ignore deprecationwarning from optimade_clien
danielhollas Sep 25, 2023
fecf535
Fix ignore
danielhollas Sep 25, 2023
0bd3b09
Cleanup
danielhollas Sep 25, 2023
20e809b
Merge branch 'master' into fix/deprecation-warnings-misc
danielhollas Oct 4, 2023
2bf2ebd
Merge branch 'master' into fix/deprecation-warnings-misc
unkcpz Oct 5, 2023
9470aff
review
danielhollas Oct 12, 2023
dcdc755
tuples all the way
danielhollas Oct 12, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
**/pyproject.toml
**/requirements*.txt

- uses: pre-commit/action@v2.0.0
- uses: pre-commit/action@v3.0.0

test-notebooks:

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:

steps:

- uses: actions/checkout@v2
- uses: actions/checkout@v3

- name: Set up Python 3.10
uses: actions/setup-python@v2
Expand Down
5 changes: 4 additions & 1 deletion aiidalab_widgets_base/data/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ def __init__(self, value=0, description="Select functional group", **kwargs):
self.style = {"description_width": "initial"}
self.layout = {"width": "initial"}
super().__init__(
value=value, description=description, options=FUNCTIONAL_GROUPS, **kwargs
value=value,
description=description,
options=tuple((key, value) for key, value in FUNCTIONAL_GROUPS.items()),
**kwargs,
)

def rotate(self, align_to=(0, 0, 1), remove_anchor=False):
Expand Down
2 changes: 1 addition & 1 deletion aiidalab_widgets_base/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ class OpenAiidaNodeInAppWidget(ipw.VBox):

def __init__(self, path_to_root="../", **kwargs):
self.path_to_root = path_to_root
self.tab = ipw.Tab(style={"description_width": "initial"})
self.tab = ipw.Tab()
self.tab_selection = ipw.RadioButtons(
options=[],
description="",
Expand Down
9 changes: 4 additions & 5 deletions aiidalab_widgets_base/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ def __init__(self, title="Progress Bar", **kwargs):
value=0,
min=0,
max=2,
step=1,
description="Progress:",
bar_style="warning", # 'success', 'info', 'warning', 'danger' or ''
orientation="horizontal",
Expand Down Expand Up @@ -537,7 +536,7 @@ def __init__(self, title="Running Job Output", **kwargs):
self.title = title
self.selection = ipw.Dropdown(
description="Select calculation:",
options={p.id: p for p in get_running_calcs(self.process)},
options=tuple((p.id, p) for p in get_running_calcs(self.process)),
style={"description_width": "initial"},
)
self.output = CalcJobOutputWidget()
Expand All @@ -550,9 +549,9 @@ def update(self):
return
with self.hold_trait_notifications():
old_label = self.selection.label
self.selection.options = {
str(p.id): p for p in get_running_calcs(self.process)
}
self.selection.options = tuple(
(str(p.id), p) for p in get_running_calcs(self.process)
)
# If the selection remains the same.
if old_label in self.selection.options:
self.label = old_label # After changing options trait, the label and value traits might change as well.
Expand Down
12 changes: 6 additions & 6 deletions aiidalab_widgets_base/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import io
import pathlib
import tempfile
from collections import OrderedDict

import ase
import ipywidgets as ipw
Expand Down Expand Up @@ -98,7 +97,10 @@ def __init__(

# Store format selector.
data_format = ipw.RadioButtons(
options=self.SUPPORTED_DATA_FORMATS, description="Data type:"
options=tuple(
(key, value) for key, value in self.SUPPORTED_DATA_FORMATS.items()
),
description="Data type:",
)
tl.link((data_format, "label"), (self, "node_class"))

Expand Down Expand Up @@ -658,17 +660,15 @@ def search(self, _=None):
matches = {n[0] for n in qbuild.iterall()}
matches = sorted(matches, reverse=True, key=lambda n: n.ctime)

options = OrderedDict()
options[f"Select a Structure ({len(matches)} found)"] = False

options = [(f"Select a Structure ({len(matches)} found)", False)]
for mch in matches:
label = f"PK: {mch.pk}"
label += " | " + mch.ctime.strftime("%Y-%m-%d %H:%M")
label += " | " + mch.base.extras.get("formula", "")
label += " | " + mch.node_type.split(".")[-2]
label += " | " + mch.label
label += " | " + mch.description
options[label] = mch
options.append((label, mch))

self.results.options = options

Expand Down
25 changes: 7 additions & 18 deletions aiidalab_widgets_base/viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,32 +34,22 @@ def registration_decorator(widget):
return registration_decorator


def viewer(obj, downloadable=True, **kwargs):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't be passing this attribute to the viewers in general because most of them do not have it, and then pass it up to the ipywidget parent which results in warnings. Since this parameter is only used in DictViewer and FolderDataViewer I think it is okay to get rid of it, but please correct me if I am wrong.

Copy link
Member

Choose a reason for hiding this comment

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

The problem maybe the viewer return from calling viewer directly will not be exactly the same as DictViewer. Where the downloadable can not pass to the viewer anymore.
I think it is fine to get rid of it but for the DictViewer the downloadable need to get from kwargs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I am not sure I got your comment. Do you want me to change anything?

The problem maybe the viewer return from calling viewer directly will not be exactly the same as DictViewer. Where the downloadable can not pass to the viewer anymore.

Yes. But other viewers may have other input parameters and we don't support them either, so I don't see why Downloadable should be privileged in any way.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I am not sure I got your comment. Do you want me to change anything?

Never mind, I misunderstood the usage of the viewer in the first place. I was asking for changing __init__ of DictViewer, instead of passing downloadable but passing downloadable through kwargs.

It is not necessary, and your change is all good. The expected way of using the viewer function in the AiiDAlab is by AiidaNodeViewecWidget, which only passes the orm.Node without any parameters.

def viewer(obj, **kwargs):
"""Display AiiDA data types in Jupyter notebooks.

:param downloadable: If True, add link/button to download the content of displayed AiiDA object.
:type downloadable: bool

Returns the object itself if the viewer wasn't found."""
if not isinstance(obj, orm.Node): # only working with AiiDA nodes
warnings.warn(
f"This viewer works only with AiiDA objects, got {type(obj)}", stacklevel=2
)
return obj

try:
if obj.node_type in AIIDA_VIEWER_MAPPING:
_viewer = AIIDA_VIEWER_MAPPING[obj.node_type]
except KeyError as exc:
if obj.node_type in str(exc):
warnings.warn(
f"Did not find an appropriate viewer for the {type(obj)} object. Returning the object "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't find this warning particularly helpful since it is kinda self-evident. Also for Base types like Int it's perfectly okay not to have a dedicated viewer.

"itself.",
stacklevel=2,
)
return obj
raise
return _viewer(obj, **kwargs)
else:
return _viewer(obj, downloadable=downloadable, **kwargs)
# No viewer registered for this type, return object itself
return obj


class AiidaNodeViewWidget(ipw.VBox):
Expand Down Expand Up @@ -294,12 +284,11 @@ def change_supercell(_=None):

# 3. Camera switcher
camera_type = ipw.ToggleButtons(
options={"Orthographic": "orthographic", "Perspective": "perspective"},
options=[("Orthographic", "orthographic"), ("Perspective", "perspective")],
description="Camera type:",
value=self._viewer.camera,
layout={"align_self": "flex-start"},
style={"button_width": "115.5px"},
orientation="vertical",
)

def change_camera(change):
Expand Down Expand Up @@ -482,7 +471,7 @@ def _download_tab(self):
)

# 4. Render a high quality image
self.render_btn = ipw.Button(description="Render", icon="fa-paint-brush")
self.render_btn = ipw.Button(description="Render", icon="paint-brush")
self.render_btn.on_click(self._render_structure)
self.render_box = ipw.VBox(
children=[ipw.Label("Render an image with POVRAY:"), self.render_btn]
Expand Down
24 changes: 24 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,27 @@ requires = [
"wheel"
]
build-backend = "setuptools.build_meta"

[tool.pytest.ini_options]
filterwarnings = [
'error',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of the warnings that I cleaned up in this PR uncovered actual bugs so imo it's good to just turn all warnings into errors so that we keep it clean. If it turned out to be too much chore we can always revert this decision.

It is a bit unfortunate that we need so many 'ignore's here but hopefully we'll reduce this as we update our third-party dependencies.

'ignore::DeprecationWarning:bokeh.core.property.primitive',
'ignore:Creating AiiDA configuration:UserWarning:aiida',
'ignore:crystal system:UserWarning:ase.io.cif',
'ignore::DeprecationWarning:ase.atoms',
# TODO: This comes from a transitive dependency of optimade_client
# Remove this when this issue is addressed:
# https://github.com/CasperWA/ipywidgets-extended/issues/85
'ignore:metadata.*trait.*<traitlets.traitlets.Unicode object:DeprecationWarning:traitlets',
# For some reason we get this error, perhaps because we do
# not unload the AiiDA profile? Let's ignore this for now.
# E pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function Popen.__del__>
# E Traceback (most recent call last):
# E File "/opt/conda/lib/python3.9/subprocess.py", line 1052, in __del__
# E _warn("subprocess %s is still running" % self.pid,
# E ResourceWarning: subprocess 382685 is still running
'ignore:Exception ignored in:pytest.PytestUnraisableExceptionWarning:_pytest',
'ignore::DeprecationWarning:jupyter_client',
'ignore::DeprecationWarning:selenium',
'ignore::DeprecationWarning:pytest_selenium',
]
20 changes: 12 additions & 8 deletions tests/test_databases.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import ase

import aiidalab_widgets_base as awb


def test_cod_query_widget():
"""Test the COD query widget."""
from aiidalab_widgets_base import CodQueryWidget

widget = CodQueryWidget()
widget = awb.CodQueryWidget()

# Enter the query string.
widget.inp_elements.value = "Ni Ti"

# Run the query.
widget._on_click_query()

# Select on of the results.
# Select one of the results.
# TODO: Select a different structure to get rid of the ASE warning:
# "ase/io/cif.py:401: UserWarning: crystal system 'cubic' is not interpreted
# for space group 'Pm-3m'. This may result in wrong setting!"
widget.drop_structure.label = "NiTi (id: 1100132)"

# Check that the structure was loaded.
Expand All @@ -23,9 +27,8 @@ def test_cod_query_widget():

def test_optimade_query_widget():
"""Test the OPTIMADE query widget."""
from aiidalab_widgets_base import OptimadeQueryWidget

widget = OptimadeQueryWidget()
widget = awb.OptimadeQueryWidget()

# At the present state I cannot check much. Most of the variables are locals of the __init__ method.

Expand All @@ -34,14 +37,15 @@ def test_optimade_query_widget():

def test_computational_resources_database_widget():
"""Test the structure browser widget."""
from aiidalab_widgets_base.databases import ComputationalResourcesDatabaseWidget

# Initiate the widget with no arguments.
widget = ComputationalResourcesDatabaseWidget()
widget = awb.databases.ComputationalResourcesDatabaseWidget()
assert "merlin.psi.ch" in widget.database

# Initialize the widget with default_calc_job_plugin="cp2k"
widget = ComputationalResourcesDatabaseWidget(default_calc_job_plugin="cp2k")
widget = awb.databases.ComputationalResourcesDatabaseWidget(
default_calc_job_plugin="cp2k"
)
assert (
"merlin.psi.ch" not in widget.database
) # Merlin does not have CP2K installed.
Expand Down
42 changes: 14 additions & 28 deletions tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def return_inputs():
@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_inputs_widget(generate_calc_job_node):
"""Test ProcessInputWidget with a simple `CalcJobNode`"""
from aiidalab_widgets_base.process import ProcessInputsWidget

process = generate_calc_job_node(
inputs={
Expand All @@ -44,9 +43,9 @@ def test_process_inputs_widget(generate_calc_job_node):
)

# Test the widget can be instantiated with empty inputs
process_input_widget = ProcessInputsWidget()
process_input_widget = awb.ProcessInputsWidget()

process_input_widget = ProcessInputsWidget(process=process)
process_input_widget = awb.ProcessInputsWidget(process=process)
input_dropdown = process_input_widget._inputs

assert "parameters" in [key for key, _ in input_dropdown.options]
Expand All @@ -64,13 +63,11 @@ def test_process_inputs_widget(generate_calc_job_node):
@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_outputs_widget(multiply_add_completed_workchain):
"""Test ProcessOutputWidget with a simple `WorkChainNode`"""
from aiidalab_widgets_base.process import ProcessOutputsWidget

# Test the widget can be instantiated with empty inputs
widget = ProcessOutputsWidget()
widget = awb.ProcessOutputsWidget()

# Test the widget can be instantiated with a process
widget = ProcessOutputsWidget(process=multiply_add_completed_workchain)
widget = awb.ProcessOutputsWidget(process=multiply_add_completed_workchain)

# Simulate output selection.
widget.show_selected_output(change={"new": "result"})
Expand All @@ -79,17 +76,15 @@ def test_process_outputs_widget(multiply_add_completed_workchain):
@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_follower_widget(multiply_add_process_builder_ready, daemon_client):
"""Test ProcessFollowerWidget with a simple `WorkChainNode`"""
from aiidalab_widgets_base.process import ProcessFollowerWidget

# Test the widget can be instantiated with empty inputs
widget = ProcessFollowerWidget()
widget = awb.ProcessFollowerWidget()

if daemon_client.is_daemon_running:
daemon_client.stop_daemon(wait=True)
process = engine.submit(multiply_add_process_builder_ready)

# Test the widget can be instantiated with a process
widget = ProcessFollowerWidget(process=process)
widget = awb.ProcessFollowerWidget(process=process)

daemon_client.start_daemon()

Expand All @@ -104,18 +99,16 @@ def test_process_report_widget(
multiply_add_process_builder_ready, daemon_client, await_for_process_completeness
):
"""Test ProcessReportWidget with a simple `WorkChainNode`"""
from aiidalab_widgets_base.process import ProcessReportWidget

# Test the widget can be instantiated with empty inputs
ProcessReportWidget()
awb.ProcessReportWidget()

# Stopping the daemon and submitting the process.
if daemon_client.is_daemon_running:
daemon_client.stop_daemon(wait=True)
process = engine.submit(multiply_add_process_builder_ready)

# Test the widget can be instantiated with a process
widget = ProcessReportWidget(process=process)
widget = awb.ProcessReportWidget(process=process)
assert (
widget.value == "No log messages recorded for this entry"
) # No report produced yet.
Expand Down Expand Up @@ -218,26 +211,22 @@ def test_running_calcjob_output_widget(generate_calc_job_node):
}
)

# Test the widget can be instantiated with a process
RunningCalcJobOutputWidget(calculation=process)
widget = RunningCalcJobOutputWidget()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The RunninngCalcJobOutputWidget initializer does not have calculation as its input. This code was likely copy-pasted. This API is a bit unconsistent, since many other widgets allow passing the traitlet via constructor. But that's for another PR.

widget.process = process


@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_list_widget(multiply_add_completed_workchain):
"""Test ProcessListWidget with a simple `WorkChainNode`"""
from aiidalab_widgets_base.process import ProcessListWidget

ProcessListWidget()
awb.ProcessListWidget()


@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_monitor(
multiply_add_process_builder_ready, daemon_client, await_for_process_completeness
):
"""Test ProcessMonitor with a simple `WorkChainNode`"""
from aiidalab_widgets_base.process import ProcessMonitor

ProcessMonitor()
awb.ProcessMonitor()

# Stopping the daemon and submitting the process.
if daemon_client.is_daemon_running:
Expand All @@ -250,7 +239,7 @@ def f():
nonlocal test_variable
test_variable = True

widget = ProcessMonitor(value=process.uuid, callbacks=[f])
widget = awb.ProcessMonitor(value=process.uuid, callbacks=[f])

# Starting the daemon and waiting for the process to complete.
daemon_client.start_daemon()
Expand All @@ -265,7 +254,4 @@ def f():
@pytest.mark.usefixtures("aiida_profile_clean")
def test_process_nodes_tree_widget(multiply_add_completed_workchain):
"""Test ProcessNodesTreeWidget with a simple `WorkChainNode`"""

from aiidalab_widgets_base.process import ProcessNodesTreeWidget

ProcessNodesTreeWidget(value=multiply_add_completed_workchain.uuid)
awb.ProcessNodesTreeWidget(value=multiply_add_completed_workchain.uuid)
4 changes: 1 addition & 3 deletions tests/test_viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ def test_pbc_structure_data_viewer(structure_data_object):

import ase

from aiidalab_widgets_base import viewers

# Prepare a structure with periodicity xy
ase_input = ase.Atoms(
symbols="Li2",
Expand Down Expand Up @@ -59,7 +57,7 @@ def test_several_data_viewers(


@pytest.mark.usefixtures("aiida_profile_clean")
def test_structure_data_viwer(structure_data_object):
def test_structure_data_viewer(structure_data_object):
v = viewers.viewer(structure_data_object)
assert isinstance(v, viewers.StructureDataViewer)

Expand Down