Skip to content

Update use of Actor rendering class #101

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 1 commit into from
Jun 13, 2025
Merged

Conversation

kmorel
Copy link
Collaborator

@kmorel kmorel commented May 20, 2025

When the rendering classes were originally created, Actor accepted the cell set, coordinate system, and field independently. More recently (but not that recently), new constructors were made that accept a DataSet object and pulls these out for you. This is generally a better way to construct actors, so update the documentation and testing code to follow this convention.

@kmorel kmorel requested a review from dpugmire May 20, 2025 18:20
@kmorel kmorel force-pushed the update-actor-usage branch from 470d837 to 4fde484 Compare May 20, 2025 18:24
@kmorel
Copy link
Collaborator Author

kmorel commented May 20, 2025

@dpugmire I usually lean on you for the rendering code in Viskores. Can you do a review of these changes. The lines are mostly documentation and style. I also added some new constructors to Actor that do not require specifying the coordinate system.

@kmorel
Copy link
Collaborator Author

kmorel commented Jun 6, 2025

@dpugmire ping

1 similar comment
@kmorel
Copy link
Collaborator Author

kmorel commented Jun 13, 2025

@dpugmire ping

@kmorel kmorel force-pushed the update-actor-usage branch from a3cd2a0 to 5eb35fe Compare June 13, 2025 14:55
When the rendering classes were originally created, `Actor` accepted
the cell set, coordinate system, and field independently. More
recently (but not that recently), new constructors were made that
accept a `DataSet` object and pulls these out for you. This is generally
a better way to construct actors, so update the documentation and
testing code to follow this convention.

Also added new `Actor` constructors that do not require passing
the name of the coordinate system. This is for the common use case
where each dataset has exactly one coordinate system.
@kmorel kmorel force-pushed the update-actor-usage branch 2 times, most recently from 3438fb4 to bd84882 Compare June 13, 2025 16:35
Copy link
Collaborator

@dpugmire dpugmire left a comment

Choose a reason for hiding this comment

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

Looks good.

@kmorel kmorel merged commit 55e7193 into Viskores:main Jun 13, 2025
15 checks passed
@kmorel kmorel deleted the update-actor-usage branch June 13, 2025 19:55
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