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

Update Channels XML comment #1466

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Update Channels XML comment #1466

merged 2 commits into from
Oct 23, 2023

Conversation

kmgallahan
Copy link
Contributor

Indicate that only color channels are returned.

Description

I was scratching my head as to why ChannelCount and Channels.Count() weren't matching until I viewed the source.

Indicate that only color channels are returned
@dlemstra
Copy link
Owner

Thanks again for helping out with improving the documentation. The property also returns meta channels so I wonder if we could use another word than color.

@kmgallahan
Copy link
Contributor Author

kmgallahan commented Oct 22, 2023

Ah yes, I missed that. So colors plus:

MetaPixelChannels = 10,

and anything else added later below 64, but excluding:

IndexPixelChannel = 5,
ReadMaskPixelChannel = 6,
WriteMaskPixelChannel = 7,
MetaPixelChannel = 8, /* deprecated */
CompositeMaskPixelChannel = 9,
IntensityPixelChannel = MaxPixelChannels,  /* ???? */
CompositePixelChannel = MaxPixelChannels,  /* ???? */
SyncPixelChannel = MaxPixelChannels+1      /* not a real channel */

Not sure how to phrase that. Do you remember what drove the decision to filter these channels? That could inform the description.

@dlemstra
Copy link
Owner

dlemstra commented Oct 22, 2023

I did not include the index and mask channels because they are used for a specific functionality in the library and don't really change how the image looks? And the other ones are either deprecated or only used on the command line. I really appreciate you making this more clear for future users.

@kmgallahan
Copy link
Contributor Author

Ok, I just added metadata. I was considering adding Excludes index and mask channels, but I don't think listing exclusions is particularly common in API docs.

@dlemstra dlemstra merged commit 3ca8e97 into dlemstra:main Oct 23, 2023
26 checks passed
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