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

rework convert_attribute docstring #2507

Merged
merged 4 commits into from
Jan 12, 2023
Merged

rework convert_attribute docstring #2507

merged 4 commits into from
Jan 12, 2023

Conversation

jw3126
Copy link
Contributor

@jw3126 jw3126 commented Dec 18, 2022

supersedes #2504

julia> Makie.convert_attribute(:black, key"color"())
RGBA{Float32}(0.0f0,0.0f0,0.0f0,1.0f0)
```
"""
function convert_attribute end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a couple of questions about this function. Is this an internal function? Is the user supposed to overload it? If so are there typical cases, when a user might want to overload this?

Copy link
Contributor Author

@jw3126 jw3126 Dec 18, 2022

Choose a reason for hiding this comment

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

For instance I might have a custom recipe particles that takes attributes
photoncolor and electroncolor.
Should I overload

MakieCore.convert_attributes(value, ::key"photoncolor", ::key"phsp") = convert_attributes(value, ::key"color")
MakieCore.convert_attributes(value, ::key"electroncolor", ::key"phsp") = convert_attributes(value, ::key"color")

?

Copy link
Member

Choose a reason for hiding this comment

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

No, this is only for atomic plot types (like scatter, mesh, etc) and gets called in the backend.
You can use to_color in your recipe for such cases...
Overloading is only useful, if you have your own custom color type and want to use it directly as a color attribute!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks! I think this can be merged. Before this PR, the docstring is really confusing:

help?> convert_attribute
search: convert_attribute

  to_color(color)


  Converts a color symbol (e.g. :blue) to a color RGBA.

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  to_colormap(cm)


  Converts a colormap cm symbol/string (e.g. :Spectral) to a colormap RGB array.

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  `AbstractVector{<:AbstractFloat}` for denoting sequences of fill/nofill. e.g.


  [0.5, 0.8, 1.2] will result in 0.5 filled, 0.3 unfilled, 0.4 filled. 1.0 unit is one linewidth!

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  A `Symbol` equal to `:dash`, `:dot`, `:dashdot`, `:dashdotdot`


  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  to_volume_algorithm(b, x)


  Enum values: IsoValue Absorption MaximumIntensityProjection AbsorptionRGBA AdditiveRGBA IndexedAbsorptionRGBA

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  Symbol/String: iso, absorption, mip, absorptionrgba, indexedabsorption

@SimonDanisch SimonDanisch merged commit 50c325b into MakieOrg:master Jan 12, 2023
@SimonDanisch
Copy link
Member

Thanks!

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.

2 participants