Skip to content

vis + gate algo refactor#34

Merged
nLevin13 merged 21 commits intomasterfrom
pytest-evaluation
Jan 18, 2021
Merged

vis + gate algo refactor#34
nLevin13 merged 21 commits intomasterfrom
pytest-evaluation

Conversation

@dekahaedra
Copy link
Copy Markdown
Member

No description provided.

@dekahaedra dekahaedra requested a review from nLevin13 January 4, 2021 08:17
Comment thread perception/tasks/gate/GateCenterAlgo.py Outdated
Comment thread perception/tasks/gate/GatePerceiver.py Outdated
Comment thread perception/tasks/gate/GateSegmentationAlgoA.py Outdated
from typing import Tuple
import sys
import os
sys.path.append(os.path.dirname(__file__))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same thing

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see this line deleted. Is this needed for some reason still?

Comment thread perception/tasks/gate/GateSegmentationAlgoC.py Outdated
Comment thread requirements.txt Outdated
@AVSurfer123
Copy link
Copy Markdown
Member

Also why are there 3 different algorithms, A, B, C? is it just older versions? Probably should be renamed better

Copy link
Copy Markdown
Member

@nLevin13 nLevin13 left a comment

Choose a reason for hiding this comment

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

I noticed that this branch is several commits behind master, so there's a features I added to vis.py that aren't in your version, namely the pyplot integration. Can you merge from master first then apply your changes? Other than that, it seems good

Comment thread perception/vis/vis.py Outdated
Copy link
Copy Markdown
Member

@AVSurfer123 AVSurfer123 left a comment

Choose a reason for hiding this comment

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

Just 2 more small deletions and its good to go

@@ -8,10 +8,10 @@
#############################################################################

sys.path.insert(0, '../background_removal')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

delete this line

from typing import Tuple
import sys
import os
sys.path.append(os.path.dirname(__file__))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see this line deleted. Is this needed for some reason still?

@nLevin13
Copy link
Copy Markdown
Member

We can remove (archive) detectGate, threshTest, and threshslider

@nLevin13 nLevin13 merged commit daa6c6d into master Jan 18, 2021
@nLevin13 nLevin13 deleted the pytest-evaluation branch January 18, 2021 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants