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

Refactor dataset retriever to isolate CLI functions #294

Merged
merged 3 commits into from
Aug 27, 2023

Conversation

neubig
Copy link
Collaborator

@neubig neubig commented Aug 27, 2023

Description

In preparation for preparing our notebook demo (#289), it would be nice to split off the functions that require the CLI so we can replace them with more notebook-native ways of doing things. This PR starts by doing some refactoring to make the interface to the retriever a bit more cleanly separated.

References

Blocked by

  • NA

"""Get a yes/no answer from the user via stdin."""
y_n = str(input())
return not (y_n.strip() == "" or y_n.strip().lower() == "n")
logger = logging.getLogger(__name__)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I should change this in all the other components. 😅

Copy link
Collaborator

@zhaochenyang20 zhaochenyang20 left a comment

Choose a reason for hiding this comment

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

A quick question:

Adding _ to some function means that the function is private, right?

@neubig neubig requested a review from zhaochenyang20 August 27, 2023 14:18
Copy link
Collaborator

@zhaochenyang20 zhaochenyang20 left a comment

Choose a reason for hiding this comment

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

Go ahead!

@neubig neubig mentioned this pull request Aug 27, 2023
@zhaochenyang20 zhaochenyang20 merged commit 159b57f into main Aug 27, 2023
@zhaochenyang20 zhaochenyang20 deleted the refactor_dataset_retriever branch August 27, 2023 14:28
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.

Remove mandatory command line entry from dataset retriever
2 participants