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

Feat/yuanrui/enhanced images #135

Merged
merged 36 commits into from
Oct 21, 2024
Merged

Conversation

zhang-yuanrui
Copy link
Collaborator

Consolidate the first version of the enhanced image module.

What it can accomplish:

  1. Given a vtk polydata and a certain variable in the polydata, it can generate an enhanced image.
  2. Given a DPF model and DPF field object, it can generate an enhanced image.
  3. The enhanced image can be a TIFF file on disk, or a PIL Image object in memory.
  4. The rendering and value pass processes are broken down by phase, and the essential RGB buffer, pick data buffer and variable data buffer are also generated independently.

Known problems:

  1. Only support scalar data for 1 model and 1 field.
  2. The JSON metadata part_name, colorby_var and pal_id entries can be further enhanced.

Potential next-step development:

  1. Make it a class as the incoming requests increase. Currently, it is only a group of functions. But write it in a class can be long-term preferrable.
  2. Grab as much information as possible from DPF inputs to improve usability. For instance, try and test if part_name can be retrieved from DPF model.
  3. Refine the part-var linkage so that enhanced image can support a list of pairs.

@zhang-yuanrui zhang-yuanrui self-assigned this Oct 4, 2024
@zhang-yuanrui zhang-yuanrui marked this pull request as ready for review October 4, 2024 20:13
@margalva
Copy link
Collaborator

margalva commented Oct 8, 2024

@zhang-yuanrui I don't want to add dpf and vtk as requirements for pydynamicreporting. Please see how report_download_pdf.py handles a similar situation with PyQt5. If the module exists, then methods are defined. If it doesn't exist, then the module does nothing. I'd like to see something similar here.

I'd also like to see some tests added in the tests/ directory for these methods. You should add in the pyproject.toml the requirements for the extra modules only for tests (and maybe development), and create a few tests for these methods. Let me know if you need help here.

@randallfrank
Copy link
Collaborator

I think my biggest comment here is the lack of example(s) and test cases. One can work around the dependency issues for tests that @margalva calls out using the yaml test deps target. My other big concern is the impact on sphinx documentation. There are a number of "internal" functions exposed here that one would like to avoid getting into the docs. Hiding them as '_' methods in a class would help. Consider providing a simple class with a pure VTK interface and the appropriate methods hidden. The DPF methods can move over into DPF and leverage the VTK interface from there, streamlining the dependencies.

@zhang-yuanrui
Copy link
Collaborator Author

I think my biggest comment here is the lack of example(s) and test cases. One can work around the dependency issues for tests that @margalva calls out using the yaml test deps target. My other big concern is the impact on sphinx documentation. There are a number of "internal" functions exposed here that one would like to avoid getting into the docs. Hiding them as '_' methods in a class would help. Consider providing a simple class with a pure VTK interface and the appropriate methods hidden. The DPF methods can move over into DPF and leverage the VTK interface from there, streamlining the dependencies.

@randallfrank What is the yaml test deps target?

@zhang-yuanrui zhang-yuanrui marked this pull request as draft October 9, 2024 14:18
@zhang-yuanrui zhang-yuanrui marked this pull request as ready for review October 18, 2024 18:23
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
@zhang-yuanrui zhang-yuanrui enabled auto-merge (squash) October 21, 2024 16:45
@zhang-yuanrui zhang-yuanrui merged commit db039d8 into main Oct 21, 2024
23 checks passed
@zhang-yuanrui zhang-yuanrui deleted the feat/yuanrui/enhanced_images branch October 21, 2024 19:56
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