Skip to content

Fix the Quickstart tutorial and make the docs refer to this repo instead of a fork#20

Merged
lorenzoh merged 10 commits into
FluxML:masterfrom
dave7895:master
Mar 27, 2021
Merged

Fix the Quickstart tutorial and make the docs refer to this repo instead of a fork#20
lorenzoh merged 10 commits into
FluxML:masterfrom
dave7895:master

Conversation

@dave7895

@dave7895 dave7895 commented Mar 23, 2021

Copy link
Copy Markdown
Contributor

This should fix the erroring tests, as I removed the reference to a local DLPipelines or at least change the location of/reason for the error.

@lorenzoh

lorenzoh commented Mar 25, 2021

Copy link
Copy Markdown
Member

Thanks for the contribution! Could you separate the changes so that the pull request is atomic, i.e. doesn't do many things at once? Currently it contains the following changes:

  • Documentation fixes like links and setup instructions. Should be in a separate PR.
  • Fixing the DLPipelines dependency in Manifest.toml. You can take this out, as it will be fixed with a new DLPipelines release.
  • Additional encode method. You can take this out, as it will be fixed with a new DLPipelines release.
  • Removing validbsfactor. You can take this out, as it will be fixed with a new DLPipelines release.

Let me know if you have any questions :) The help is much appreciated!

dave7895 and others added 4 commits March 25, 2021 16:15
now uses GitHub version instead of local
testing and tutorial errored out for ImageClassification task. An `encode` function was added, equivalent to the one already existing for ImageSegmentation
@dave7895

Copy link
Copy Markdown
Contributor Author

I hope this satisfies your request.

Removed extra `encode` method. This was fixed in DLPipelines.jl v0.2.0
@lorenzoh

Copy link
Copy Markdown
Member

I've gone ahead and removed the encode method as it was fixed with the release of DLPipelines.jl v0.2.0.

@dave7895

Copy link
Copy Markdown
Contributor Author

So revert everything except the docs fix?

@lorenzoh

Copy link
Copy Markdown
Member

Should be the case now, I'll merge once the checks have run through 👍

@dave7895

Copy link
Copy Markdown
Contributor Author

I messed up the commit history. Should I clean it up and force push again?

@lorenzoh

Copy link
Copy Markdown
Member

You don't need to, I'll squash the commits down into one when I merge

@lorenzoh lorenzoh merged commit 85eb62c into FluxML:master Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants