Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

#229 Fix identification on LUNA #252

Merged
merged 1 commit into from
Nov 28, 2017

Conversation

WGierke
Copy link
Contributor

@WGierke WGierke commented Nov 26, 2017

I fixed the identification of LUNA nodules as far as it gets on my machine. This means that currently all tests in src/tests/test_identification.py are failing for me since I don't have 21 GB of RAM (if RUN_SLOW_TESTS is set to true). Maybe someone else can execute them and send me tracebacks for further error investigation?

Description

The problem was that sometimes the direct path to a MHD file was handed to the function instead of the path to the directory. The made changes should support now both alternatives.

Reference to official issue

This addresses #229 - identification test(s) should pass for luna scans.

How Has This Been Tested?

I added the test given in #229 .

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby
Copy link
Contributor

lamby commented Nov 27, 2017

Hey, can you clarify/fixup the "WIP" commit, and perhaps rebase to avoid that merge commit? *g*


docker-compose -f local.yml run prediction pytest -rsx src/tests/test_identification.py

docker-compose -f local.yml run prediction pytest -rsx
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm?

@WGierke
Copy link
Contributor Author

WGierke commented Nov 27, 2017

@lamby Done. I changed test_docker for faster testing purposes, but that has been reverted again :) Shouldn't have commited that.

@lamby
Copy link
Contributor

lamby commented Nov 27, 2017

I don't have 21 GB of RAM

Amateur!

@reubano reubano merged commit e948d7f into drivendataorg:master Nov 28, 2017
@WGierke
Copy link
Contributor Author

WGierke commented Nov 28, 2017

I really appreciate your offer to run the identification tests on your machine and to send me possible occurring stack traces @lamby ! 😁

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants