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

basic raw volume data support #1278

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

aliaksei-chareshneu
Copy link
Collaborator

@aliaksei-chareshneu aliaksei-chareshneu commented Sep 30, 2024

Description

Actions

  • Added description of changes to the [Unreleased] section of CHANGELOG.md
  • Updated headers of modified files
  • Added my name to package.json's contributors
  • (Optional but encouraged) Improved documentation in docs

@dsehnal dsehnal requested a review from midlik September 30, 2024 14:46
Copy link
Collaborator

@midlik midlik left a comment

Choose a reason for hiding this comment

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

Great, thank you Aliaksei.
I didn't test this, just went through the code and added a few comments.

src/extensions/mvs/tree/mvs/mvs-builder.ts Outdated Show resolved Hide resolved
src/extensions/mvs/tree/molstar/molstar-tree.ts Outdated Show resolved Hide resolved
src/extensions/mvs/tree/molstar/conversion.ts Outdated Show resolved Hide resolved
src/extensions/mvs/load.ts Outdated Show resolved Hide resolved
src/extensions/mvs/load.ts Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -25,6 +25,7 @@ Note that since we don't clearly distinguish between a public and private interf
- `native`, no changes
- Add `CustomProperty.Context.errorContext` to support reporting errors during loading of custom properties (#1254)
- Use in MolViewSpec extension
- Add basic support for volumetric data
Copy link
Member

Choose a reason for hiding this comment

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

this is just for MVS, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is just for MVS, right?

Yes, you are right, I will edit the description to highlight that this is just for MVS. I apologise for missing this point initially.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is just for MVS, right?

Fixed in ba42726. Thanks again for the catch.

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.

3 participants