-
Notifications
You must be signed in to change notification settings - Fork 18
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
Changes from all commits
dee6b62
106c913
e29dd56
2d7daab
b6ee99c
ff053d4
0a10d8c
9ed6fbc
1f75cb8
e655614
c3f0c5b
1644d8f
3bcead9
b98285d
144f24f
7896eeb
1167ab6
339dd61
fc58864
fecf535
0bd3b09
20e809b
2bf2ebd
9470aff
dcdc755
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,32 +34,22 @@ def registration_decorator(widget): | |
return registration_decorator | ||
|
||
|
||
def viewer(obj, downloadable=True, **kwargs): | ||
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 " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
"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): | ||
|
@@ -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): | ||
|
@@ -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] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,3 +4,27 @@ requires = [ | |
"wheel" | ||
] | ||
build-backend = "setuptools.build_meta" | ||
|
||
[tool.pytest.ini_options] | ||
filterwarnings = [ | ||
'error', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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={ | ||
|
@@ -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] | ||
|
@@ -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"}) | ||
|
@@ -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() | ||
|
||
|
@@ -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. | ||
|
@@ -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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
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: | ||
|
@@ -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() | ||
|
@@ -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) |
There was a problem hiding this comment.
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
andFolderDataViewer
I think it is okay to get rid of it, but please correct me if I am wrong.There was a problem hiding this comment.
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 asDictViewer
. Where thedownloadable
can not pass to the viewer anymore.I think it is fine to get rid of it but for the
DictViewer
thedownloadable
need to get fromkwargs
.There was a problem hiding this comment.
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?
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I misunderstood the usage of the viewer in the first place. I was asking for changing
__init__
ofDictViewer
, instead of passingdownloadable
but passingdownloadable
throughkwargs
.It is not necessary, and your change is all good. The expected way of using the
viewer
function in the AiiDAlab is byAiidaNodeViewecWidget
, which only passes theorm.Node
without any parameters.