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

Add a tutorial on creating an inset plot #4697

Merged
merged 9 commits into from
Jan 22, 2025

Conversation

sashansamarajeewa
Copy link
Contributor

@sashansamarajeewa sashansamarajeewa commented Dec 31, 2024

Description

Fixes #3292

This PR adds a tutorial on creating an inset plot. The related issue #3292 discusses 2 approaches to address this but I opted for the initial suggestion described in #3292 (comment).

The tutorial consists of 3 main parts.

  1. Explain what is an inset plot.
  2. How to make an inset plot with an example.
  3. Explain key elements and related information.

Output image

image

Type of change

Delete options that do not apply:

  • Improving the Documentation

Checklist

  • Added or changed relevant sections in the documentation

@sashansamarajeewa sashansamarajeewa marked this pull request as ready for review December 31, 2024 10:25
@sashansamarajeewa
Copy link
Contributor Author

@ffreyer @jkrumbiegel It would be great if someone can review this PR and provide feedback :)

docs/src/tutorials/inset-plot-tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorials/inset-plot-tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorials/inset-plot-tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorials/inset-plot-tutorial.md Outdated Show resolved Hide resolved
@jkrumbiegel
Copy link
Member

jkrumbiegel commented Jan 7, 2025

I think the part with the zoom lines would be better suited for the MakieExtras docs as it describes how to use its functions, and not how to do generic operations with Makies objects. I don't want to document other packages here if I don't have to as it increases the overall burden of keeping things up to date, plus, MakieExtras is specifically a package doing nonstandard things with some internals use etc.

@sashansamarajeewa
Copy link
Contributor Author

I think the part with the zoom lines would be better suited for the MakieExtras docs as it describes how to use its functions, and not how to do generic operations with Makies objects. I don't want to document other packages here if I don't have to as it increases the overall burden of keeping things up to date, plus, MakieExtras is specifically a package doing nonstandard things with some internals use etc.

Do you suggest removing the MakieExtra package and zoom_lines function from the plot and keeping the rest?

@jkrumbiegel
Copy link
Member

jkrumbiegel commented Jan 9, 2025

Yeah probably, it's already hard to keep the docs in sync with our own package, much less if we add others as well. I'd be open to showing how to do your own zoomlines, although it's maybe a little involved for a tutorial. On the other hand it would probably be good to showcase the projection functions somewhere, if we consider them API at least. Do we, @SimonDanisch and @ffreyer?

@SimonDanisch
Copy link
Member

I think we should either port zoomlines to Makie or document it in Makie Extras and link it in the docs.
projection is considered API, albeit not a thought through and well tested one :(
CC: @aplavin

@SimonDanisch
Copy link
Member

I do wonder if project is needed, since we do also have the relative space we could plot in.

@aplavin
Copy link
Contributor

aplavin commented Jan 9, 2025

or document it in Makie Extras

All public functionality in MakieExtra.jl is documented, including zoomlines! – see the docs.
It doesn't show inset plots specifically, which I agree is a common usecase (not the only one). I guess I'll add an inset example there, thanks for the suggestion!

@ffreyer
Copy link
Collaborator

ffreyer commented Jan 9, 2025

I think project is too much of a mess to advertise it.

space = :relative is also not enough here because the area you're targeting is probably in data space.

@sashansamarajeewa
Copy link
Contributor Author

Does it make sense to point to the zoomlines documentation within this tutorial or completely remove MakieExtra package and zoom_lines function from this tutorial?

@sashansamarajeewa
Copy link
Contributor Author

sashansamarajeewa commented Jan 11, 2025

Removed MakieExtra package and zoom_lines! function from this tutorial. Instead, added information on how to create a border to mark the zoomed section.

Output image

image

@SimonDanisch
Copy link
Member

Awesome :) We could maybe still link the docs for zoom_lines?

@sashansamarajeewa
Copy link
Contributor Author

sashansamarajeewa commented Jan 13, 2025

Awesome :) We could maybe still link the docs for zoom_lines?

Added information about zoom_lines function and linked the documentation.

@sashansamarajeewa
Copy link
Contributor Author

Is it possible to build the artifacts if there are no more changes to be done?

@sashansamarajeewa
Copy link
Contributor Author

Build was failing due to missing a CHANGELOG entry. Updated CHANGELOG.md file.

Copy link
Collaborator

@ffreyer ffreyer left a comment

Choose a reason for hiding this comment

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

I think this is good to merge now. (Assuming the docs build doesn't fail)

Build was failing due to missing a CHANGELOG entry. Updated CHANGELOG.md file.

We usually don't add docs changes there but maybe it's good to have this one. May help make people aware of it.

@sashansamarajeewa
Copy link
Contributor Author

Build has been successful and I hope this will be merged 🤞

@SimonDanisch
Copy link
Member

Great work :) I've checked the uploaded artifact of the documentation and it seems to all work well!

@sashansamarajeewa
Copy link
Contributor Author

sashansamarajeewa commented Jan 22, 2025

Latest CHANGELOG.md changes were conflicting with this branch so merged it.
Let me know if there is anything else to be done before merging into master.

@SimonDanisch SimonDanisch merged commit 09e2e20 into MakieOrg:master Jan 22, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

[Docs FR] Add inset example
5 participants