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

Issue3 #25

Merged
merged 3 commits into from
Oct 9, 2017
Merged

Issue3 #25

merged 3 commits into from
Oct 9, 2017

Conversation

tvirolai
Copy link
Contributor

@tvirolai tvirolai commented Oct 9, 2017

Basic implementation for create-project and drop-project.

@tvirolai tvirolai requested a review from osma October 9, 2017 13:11
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

CatClient seems redundant here, but otherwise looks good, feel free to merge after considering my comments

annif/annif.py Outdated
print(p)

print(projects)
report = cat.indices() # This queries different indices and returns
Copy link
Member

Choose a reason for hiding this comment

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

Is this really necessary? The result seems to be just thrown away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, same thing applies here. I'll do another commit to remove these before merging.

annif/annif.py Outdated
for p in projects:
print(p)

print(projects)
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant after the previous for loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, true. I tried different ways to present the results when configuring tests. This I'll remove this.

@tvirolai tvirolai merged commit e1f278e into master Oct 9, 2017
@tvirolai tvirolai deleted the issue3 branch October 9, 2017 13:43
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.

None yet

2 participants