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

Next Release v0.4.2 #721

Merged
merged 15 commits into from
Oct 3, 2022
Merged

Next Release v0.4.2 #721

merged 15 commits into from
Oct 3, 2022

Conversation

srivarra
Copy link
Contributor

@srivarra srivarra commented Sep 21, 2022

  • small dataset loading fix
  • bumped version from 0.4.1 -> 0.4.2

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Creates a the next release 0.4.2.

How did you implement your changes

Adjusted instances of 0.4.1 to 0.4.2. Added a tiny fix for downloading datasets.

Remaining issues

Next step is to create a new release via the releases page.

@srivarra srivarra added the dependencies Pull requests that update a dependency file label Sep 21, 2022
@srivarra srivarra self-assigned this Sep 21, 2022
@srivarra srivarra marked this pull request as ready for review September 22, 2022 18:39
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Some minor tweaks.

Will you update requirements.txt to have a comment at the top that says a new release needs to be made anytime the requirements are changed?

Will you also add a comment in the developer docs explaining that code changes aren't automatically propagated into the docker, with the steps that need to be done and the code chunk they'll use. I believe @ackagel mentioned it in his docker update PR

ark/utils/data_utils.py Outdated Show resolved Hide resolved
start_docker.sh Show resolved Hide resolved
ark/utils/data_utils.py Outdated Show resolved Hide resolved
docs/_rtd/contributing.md Show resolved Hide resolved
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Alex just made a similar change in his PR that got merged in

@srivarra
Copy link
Contributor Author

Yeah I let Alex know which files to checkout from this branch since the changes to the huggingface dataset file would cause a couple of tests to fail without the adjustment on his end.

@ngreenwald
Copy link
Member

Looks good, gonna wait to see if @ackagel's -u update flag fix will require a change to the docker so that we can merge that in before putting out a new release today

@ngreenwald
Copy link
Member

Turns out the issue with the docker -u flag was that the docker image was built using the old docker file. Will you add a comment to the docker file saying that any changes will require a new release? Then we'll be good to merge this in

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Almost there!

@ngreenwald ngreenwald merged commit e362e52 into main Oct 3, 2022
@ngreenwald ngreenwald deleted the next_release_v0.4.2 branch October 3, 2022 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants