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

Create documentation about layer artists #814

Merged
merged 3 commits into from
Jan 11, 2016

Conversation

astrofrog
Copy link
Member

@ChrisBeaumont - while working on this, I ran across a couple of things in the API that would be nice to update:

  • DataViewer._container: this name is pretty obscure, and it should maybe instead be DataViewer._layer_artists or DataViewer._layer_artist_container.
  • Maybe the layer artist container should have a consistent name between clients and widgets (at the moment it's .artists vs _container for the same object, and artist_container in some kwargs). What about .layer_artists, and make it public for both clients and widgets?
  • I have a mental block when I run across instances of the layer variable which really is the layer data. Would you have any objections to me renaming layer to layer_data? So then we'd never have just 'layer' - either layer artist or layer data.

What do you think?

Side note: at the moment, we don't really talk about the possibility of using clients in these docs - and that's because in principle one doesn't have to use a separate client class. Before writing docs on how to better separate client functionality and GUI, I think it would be good to converge on a solution in #775 first, so adding details of clients to the page here is low priority for now in my view.

@PennyQ - just for info, I've started working on this, but it's by no means complete. I'll work on it more over the next couple of days (in my spare time).

@ChrisBeaumont
Copy link
Member

Doc changes LGTM.

DataViewer._container: this name is pretty obscure, and it should maybe instead be DataViewer._layer_artists or DataViewer._layer_artist_container.

DataViewer._layer_artist_container sounds good

I have a mental block when I run across instances of the layer variable which really is the layer data. Would you have any objections to me renaming layer to layer_data?

I can't remember, but doesn't layer sometimes refer to a Subset as well? In that case layer_data sounds misleading to my ears (as I recall I think I started using layer as an alias for subset or data, but I agree this isn't very obvious)

@astrofrog
Copy link
Member Author

I can't remember, but doesn't layer sometimes refer to a Subset as well? In that case layer_data sounds misleading to my ears (as I recall I think I started using layer as an alias for subset or data, but I agree this isn't very obvious)

Indeed it can refer to a subset. I'll try and think of whether there is a clearer name that would better encapsulate this, but otherwise I'll leave it as-is.

@astrofrog astrofrog changed the title WIP: Create documentation about layer artists Create documentation about layer artists Jan 11, 2016
@astrofrog
Copy link
Member Author

I'll go ahead and merge this, and will continue working on the docs for the Qt data viewer in future (but what's added in this PR is already fine).

astrofrog added a commit that referenced this pull request Jan 11, 2016
Create documentation about layer artists
@astrofrog astrofrog merged commit c4189ca into glue-viz:master Jan 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants