Skip to content

reorganize and update documentation #125

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

Merged
merged 6 commits into from
Apr 28, 2020
Merged

reorganize and update documentation #125

merged 6 commits into from
Apr 28, 2020

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Apr 18, 2020

The main change of this PR is to reorganize the documentation by splitting the manual into two parts: tutorials and package documentations. Files and assets are moved around and renamed because of this.

Several other changes:

  • [index.md] replace documentation TOC with packages list
  • [install.md]
    • advertise mosaicview as recommended method to display multiple images
    • remove the outdated troubleshooting page as image IO and display are no longer an issue.
  • [api_comparison.md] [function_reference.md]
    • update functions provided by ImageDistances and ImageQualityIndexes
    • rewrite "Image loading and saving" section
    • add TOC in function references
  • [demos] fixes warnings when image value range exceeds [0, 1]
  • remove the duplicated documentation to ImageFiltering.jl

Preview: https://juliaimages.org/previews/PR125

The main change of this PR is to reorganize the documentation by splitting the manual into two parts: tutorials and package documentations. Files and assets are moved around and renamed because of this.

Several other changes:

* [index.md] replace documentation TOC with packages list
* [install.md]
  * advertise `mosaicview` as recommended method to display multiple images
  * remove the outdated troubleshooting page as image IO and display are no longer an issue.
* [api_comparison.md] [function_reference.md]
  * update functions provided by ImageDistances and ImageQualityIndexes
  * rewrite "Image loading and saving" section
  * add TOC in function references
  * add `@ref`s to all functions in api_comparison that have references
* [demos]
  * fixes warnings when image value range exceeds [0, 1]
  * remove unnecessary imports
* remove the duplicated documentation to [ImageFiltering.jl](https://juliaimages.org/ImageFiltering.jl/stable/)
Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

Wow, thanks for doing this! I agree a reorganization is called for.

I struggle with trying to figure out the "duplicate vs link" problem for packages that provide their own documentation. Why delete ImageFiltering but keep ImageAxes, ImageFeatures, etc? I think over time I've become a bigger fan of "link," as it seems to be easier to keep things up-to-date that way. What are your thoughts?

```@contents
Pages = ["install.md", "quickstart.md", "arrays_colors.md", "conversions_views.md", "indexing.md", "imageaxes.md", "imagefiltering.md", "imagemetadata.md", "imagesegmentation.md", "function_reference.md", "api_comparison.md"]
```
The following is a list of packages that are closely related to the JuliaImages ecosystem and also maintained by members of
Copy link
Member

Choose a reason for hiding this comment

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

I like the idea here, but I worry this list will be intimidating for newcomers ("Welcome to JuliaImages! Here are 60 different packages you have to master. Good luck!"), especially if it appears on the front page. I'd rather start with a gentle introduction before getting into a long list spelling out what code goes where. Let's just do using Images until readers gain a bit of experience with JuliaImages.

Copy link
Member Author

@johnnychen94 johnnychen94 Apr 24, 2020

Choose a reason for hiding this comment

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

These are moved to a new front page of the "Pakcages" part. Instead, I added some words to explain how the whole documentation is organized and built.

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 21, 2020

Why delete ImageFiltering but keep ImageAxes, ImageFeatures, etc?

Good question. I've written some words about this but it's incomplete. Here's the idea: I'm reading the JuliaImages doc as a collection of:

  • tutorials
  • packages documentation
  • demos
  • faqs (missing)
  • developer guide (missing)

Packages documentation comes from two sources: packages that don't have independent docs(e.g., ImageDistances) and that do(e.g., ImageFiltering). I believe it's programmatically doable to collect them together and build a centralized version of docs. For example:

git clone https://github.com/JuliaImages/ImageFiltering.jl
mv ImageFiltering.jl/docs/ docs/src/packages/ImageFiltering.jl
# modify make.jl to insert docs/src/packages/ImageFiltering.jl into the manual

so that we can build the docs based on each package's docs instead of plain links.

@timholy
Copy link
Member

timholy commented Apr 21, 2020

If we host them here, I agree that building the package's own docs seems like a much better solution than "forking" the docs as is done currently. Alternatively we could make sure each package has its own docs and link those. I am not sure which is better.

WRT things missing from juliaimages' docs, Chris Foster recently pointed me to a lovely resource which you may have already seen, but if not: https://documentation.divio.com/

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 21, 2020

advantages of centralized docs:

  • fewer cross-site links so it's easier to maintain references with @ref. Or it's possible to maintain cross-package references.
  • better searching function
  • easier to advertise since it could be the only one

@timholy
Copy link
Member

timholy commented Apr 21, 2020

I'm sold! My guess is we can grab the docs without needing to clone the whole repository, similar to how Pkg.add gives you "just the code" whereas Pkg.dev gives you the entire git history, etc.

@kimikage

This comment has been minimized.

@johnnychen94
Copy link
Member Author

I guess it's because these pages were initially written in days when there isn't such a feature. I'm gradually switching to that these days; e.g., the quickstart and demos are done in that way.

@kimikage

This comment has been minimized.

and rename folders for shorter URLs
"Introduction" => joinpath("pkgs", "index.md"),
"ImageAxes.jl" => joinpath("pkgs", "axes", "index.md"),
"ImageMetaData.jl" => joinpath("pkgs", "metadata", "index.md"),
"ImageSegmentation.jl" => joinpath("pkgs", "segmentation", "index.md"),
Copy link
Member Author

@johnnychen94 johnnychen94 Apr 24, 2020

Choose a reason for hiding this comment

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

As you might have noticed, this nested folder structure contributes to a long URL, e.g., https://juliaimages.org/dev/pkgs/metadata/index.html

Copy link
Member

Choose a reason for hiding this comment

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

What problem do you see with long URLs?

Copy link
Member Author

Choose a reason for hiding this comment

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

The first commit in this PR uses a longer version: https://juliaimages.org/dev/packages/imagemetadata/index.html I then removed the image prefix and replace the folder name packages with a shorter version pkgs.

If this looks good to you, it would be better not to change the folder structure/name again in the future since this makes old URLs invalid.

Choose a reason for hiding this comment

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

As this PR shows, modularization makes it easier to access their own assets from "*.md". On the other hand, it makes it difficult to access the common assets, and @example and so on run in the directory "build". So I think the length of the local relative path is more important than the URL.
There is not much difference, though.

Choose a reason for hiding this comment

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

BTW, the assets in "src" will be copied into the "build" before running doctests. So I don't think the "src/" is needed when loading an asset. That improves the consistency between load and save.

docs/make.jl Outdated
Comment on lines 22 to 23
joinpath("tutorials", "install.md"),
joinpath("tutorials", "quickstart.md"),
Copy link
Member Author

@johnnychen94 johnnychen94 Apr 25, 2020

Choose a reason for hiding this comment

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

Should install.md and quickstart.md included by Tutorials? If the answer is yes, we might need to move https://github.com/JuliaImages/juliaimages.github.io/blob/source/docs/src/quickstart.md#L302-L342 to a better place.

Copy link
Member

Choose a reason for hiding this comment

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

For me that link doesn't direct back to the specific lines of the source, even though I bet you crafted it to do so. So I'm not sure which lines you're referring to.

Copy link
Member

Choose a reason for hiding this comment

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

But maybe install & quickstart are in a different category? Maybe those could be "Introduction" and the other "Tutorials"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied the permanent link directly from github without any change, it's weird that it doesn't work on your side.

The example of usage section is removed since it's now described in index.md https://juliaimages.org/latest/quickstart/#Examples-of-usage-1

I'm not sure how to deal with "function categories", I personally find them less useful than the API comparison. https://juliaimages.org/latest/quickstart/#Function-categories-1

Comment on lines 46 to 53
```@setup versions
using InteractiveUtils
```
```@repl versions
using Pkg, Dates
today()
versioninfo()
Pkg.status()
Copy link

@kimikage kimikage Apr 25, 2020

Choose a reason for hiding this comment

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

Is this really what we need for the top page? I think the top page should be a "welcome" page, as @timholy implies in #125 (comment).

BTW, we can find the date and the version from the top-right icon.

Choose a reason for hiding this comment

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

Another option is using

```@raw html
<details>
  <summary>Package Status</summary>
```

and

```@raw html
</details>
```

I don't really recommend it, though,

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe it is necessary to share the build information somewhere in the documentation. I've now moved this to the troubleshooting section in install.md.

we can find the date and the version from the top-right icon.

Yes but I feel putting them together more convenient to get relevant information.

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

This is such a huge help!

## after julia v1.1:
## svdfactors = svd.(eachslice(channels; dims=1))
svdfactors = (svd(channels[1,:,:]), svd(channels[2,:,:]), svd(channels[3,:,:]))
## before julia v1.1:
Copy link
Member

Choose a reason for hiding this comment

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

Nice. I also like having the explicit list first since it may be easier for some readers to understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was written when documentation is built with Julia 1.0. Now since we're building docs with a newer Julia version, we could use those new iterating tools.

docs/make.jl Outdated
Comment on lines 22 to 23
joinpath("tutorials", "install.md"),
joinpath("tutorials", "quickstart.md"),
Copy link
Member

Choose a reason for hiding this comment

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

For me that link doesn't direct back to the specific lines of the source, even though I bet you crafted it to do so. So I'm not sure which lines you're referring to.

docs/make.jl Outdated
Comment on lines 22 to 23
joinpath("tutorials", "install.md"),
joinpath("tutorials", "quickstart.md"),
Copy link
Member

Choose a reason for hiding this comment

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

But maybe install & quickstart are in a different category? Maybe those could be "Introduction" and the other "Tutorials"?

"Introduction" => joinpath("pkgs", "index.md"),
"ImageAxes.jl" => joinpath("pkgs", "axes", "index.md"),
"ImageMetaData.jl" => joinpath("pkgs", "metadata", "index.md"),
"ImageSegmentation.jl" => joinpath("pkgs", "segmentation", "index.md"),
Copy link
Member

Choose a reason for hiding this comment

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

What problem do you see with long URLs?

to load images easily. The current available backends for image files are:

* [ImageMagick.jl](https://github.com/JuliaIO/ImageMagick.jl) covers most image formats and has extra
functionality. This can be your first choice if you don't have a preference.
Copy link
Member

Choose a reason for hiding this comment

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

Someday I hope we can promote ImageIO above ImageMagick, but you're probably right to keep this first for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, this is directly copied from install.md.

https://juliaimages.org/latest/install/#Loading-your-first-image-1

details and get frustrated.

🚧 This section consists of documentations of packages that are closely related to JuliaImages ecosystem.
Most of them are also maintained by members of JuliaImages. They're ordered alphabetically so you can
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Most of them are also maintained by members of JuliaImages. They're ordered alphabetically so you can
The ones marked with a `*` are available via `using Images`, but you can also use packages individually. Below, they're grouped into broad categories, then ordered alphabetically so you can

I wondered if explaining the star at the top would be more helpful than at the bottom, but I can go either way, your choice.

Copy link
Member

Choose a reason for hiding this comment

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

These provide links to the packages themselves, which seems useful, but should we say something about aggregating the documentation here?

Copy link
Member Author

@johnnychen94 johnnychen94 Apr 25, 2020

Choose a reason for hiding this comment

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

I'm not very sure if this is wanted until we make it work. Aggregating the documentation requires some tricky path manipulation, especially for those with assets.

I prefer to leave this to the next PR that works on this, which is why I add a 🚧 in the beginning.


* [Augmentor.jl](https://github.com/Evizero/Augmentor.jl) provides several basic image augmentation
operations for image-related machine learning tasks.
* [Flux.jl](https://github.com/FluxML/Flux.jl) is a featured deep learning toolbox in Julia.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* [Flux.jl](https://github.com/FluxML/Flux.jl) is a featured deep learning toolbox in Julia.
* [Flux.jl](https://github.com/FluxML/Flux.jl) is a deep learning toolbox in Julia.

I'm not sure how to draw the boundary between "featured" and non-featured.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it was advertised together with Images in https://julialang.org/ some time ago, but now it isn't the case anymore.

and move install.md out from "Tutorials"
Copy link
Member Author

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

@timholy @kimikage Thanks for all the suggestions! I've updated the docs accordingly.

The "Getting Started" is moved outside of "Tutorials" category.

## after julia v1.1:
## svdfactors = svd.(eachslice(channels; dims=1))
svdfactors = (svd(channels[1,:,:]), svd(channels[2,:,:]), svd(channels[3,:,:]))
## before julia v1.1:
Copy link
Member Author

Choose a reason for hiding this comment

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

This was written when documentation is built with Julia 1.0. Now since we're building docs with a newer Julia version, we could use those new iterating tools.

docs/make.jl Outdated
Comment on lines 22 to 23
joinpath("tutorials", "install.md"),
joinpath("tutorials", "quickstart.md"),
Copy link
Member Author

Choose a reason for hiding this comment

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

I copied the permanent link directly from github without any change, it's weird that it doesn't work on your side.

The example of usage section is removed since it's now described in index.md https://juliaimages.org/latest/quickstart/#Examples-of-usage-1

I'm not sure how to deal with "function categories", I personally find them less useful than the API comparison. https://juliaimages.org/latest/quickstart/#Function-categories-1

"Introduction" => joinpath("pkgs", "index.md"),
"ImageAxes.jl" => joinpath("pkgs", "axes", "index.md"),
"ImageMetaData.jl" => joinpath("pkgs", "metadata", "index.md"),
"ImageSegmentation.jl" => joinpath("pkgs", "segmentation", "index.md"),
Copy link
Member Author

Choose a reason for hiding this comment

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

The first commit in this PR uses a longer version: https://juliaimages.org/dev/packages/imagemetadata/index.html I then removed the image prefix and replace the folder name packages with a shorter version pkgs.

If this looks good to you, it would be better not to change the folder structure/name again in the future since this makes old URLs invalid.

to load images easily. The current available backends for image files are:

* [ImageMagick.jl](https://github.com/JuliaIO/ImageMagick.jl) covers most image formats and has extra
functionality. This can be your first choice if you don't have a preference.
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, this is directly copied from install.md.

https://juliaimages.org/latest/install/#Loading-your-first-image-1


* [Augmentor.jl](https://github.com/Evizero/Augmentor.jl) provides several basic image augmentation
operations for image-related machine learning tasks.
* [Flux.jl](https://github.com/FluxML/Flux.jl) is a featured deep learning toolbox in Julia.
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it was advertised together with Images in https://julialang.org/ some time ago, but now it isn't the case anymore.

details and get frustrated.

🚧 This section consists of documentations of packages that are closely related to JuliaImages ecosystem.
Most of them are also maintained by members of JuliaImages. They're ordered alphabetically so you can
Copy link
Member Author

@johnnychen94 johnnychen94 Apr 25, 2020

Choose a reason for hiding this comment

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

I'm not very sure if this is wanted until we make it work. Aggregating the documentation requires some tricky path manipulation, especially for those with assets.

I prefer to leave this to the next PR that works on this, which is why I add a 🚧 in the beginning.

Comment on lines 46 to 53
```@setup versions
using InteractiveUtils
```
```@repl versions
using Pkg, Dates
today()
versioninfo()
Pkg.status()
Copy link
Member Author

Choose a reason for hiding this comment

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

I believe it is necessary to share the build information somewhere in the documentation. I've now moved this to the troubleshooting section in install.md.

we can find the date and the version from the top-right icon.

Yes but I feel putting them together more convenient to get relevant information.

@johnnychen94
Copy link
Member Author

This PR is growing too big to review and discuss. Since it's basically becoming stable, I'm merging it.

I opened two issues #127 and #128 for unsolved comments. If there's something I've missed, please feel free to open more issues.

@kimikage
Copy link

🎉

BTW, did you read the error log?

@johnnychen94
Copy link
Member Author

johnnychen94 commented Apr 28, 2020

My bad, when it happened locally I thought it was due to recent ImageMagick issue.

I'll update it later.

johnnychen94 added a commit that referenced this pull request Apr 28, 2020
johnnychen94 added a commit that referenced this pull request Apr 28, 2020
follow up clean-up to #125
johnnychen94 added a commit that referenced this pull request Apr 28, 2020
follow up clean-up to #125
@johnnychen94 johnnychen94 mentioned this pull request Jul 1, 2020
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.

3 participants