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

Automatically create summary table after scripts/setfit/run_fewshot.py #262

Merged

Conversation

tomaarsen
Copy link
Member

Closes #246

Hello!

Pull Request overview

  • Automatically create summary table after scripts/setfit/run_fewshot.py.
  • Resolve potential NameError in create_summary_table if the sample_sizes is an empty list.

Details

Note that create_summary_table.py is located one directory higher than run_fewshot.py. As a result, if we want to preserve that run_fewshot.py can be called like python run_fewshot.py from the scripts/setfit directory, then we must apply the following hack:

sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from create_summary_table import create_summary_table

As opposed to the "normal" approach of:

from scripts.create_summary_table import create_summary_table

which would only work from the repository's root directory. All of these work due to the hack:

[path]/setfit/scripts/setfit> python run_fewshot.py
[path]/setfit/scripts> python setfit/run_fewshot.py
[path]/setfit> python scripts/setfit/run_fewshot.py

Furthermore, metric_name was only defined in a loop body, and then returned after the loop. If the list over which was being looped was empty, then this would cause a NameError, as metric_name was never defined. I've avoided that by also initializing the variable before the loop.

cc: @danielkorat

  • Tom Aarsen

@tomaarsen tomaarsen added the enhancement New feature or request label Jan 9, 2023
@danielkorat
Copy link
Collaborator

Thanks @tomaarsen !
What's the status of this PR?

@tomaarsen
Copy link
Member Author

It's awaiting review/ready to merge. I tend not to merge my own PRs without someone's review.

@tomaarsen
Copy link
Member Author

That said, if you're willing and able to provide a review, I'd appreciate it :)

@danielkorat
Copy link
Collaborator

@tomaarsen Sure, it was on my to do list :)

Copy link
Collaborator

@danielkorat danielkorat left a comment

Choose a reason for hiding this comment

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

@tomaarsen Looks very good, thanks! Feel free to merge when possible.

@tomaarsen tomaarsen merged commit 174eb00 into huggingface:main Jan 22, 2023
@tomaarsen tomaarsen deleted the enhancement/auto_create_summary_table branch January 22, 2023 09:56
@tomaarsen
Copy link
Member Author

Wonderful! Thanks for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically execute scripts/create_summary_table.py to reproduce average scores from paper
2 participants