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

Potential typo in Utilities/ParaView/export-scene-macro.py #2499

Closed
dasmy opened this issue Jun 29, 2022 · 7 comments · Fixed by #2503
Closed

Potential typo in Utilities/ParaView/export-scene-macro.py #2499

dasmy opened this issue Jun 29, 2022 · 7 comments · Fixed by #2503
Labels
released Automated label

Comments

@dasmy
Copy link
Contributor

dasmy commented Jun 29, 2022

I assume, this file is deprecated. However, while trying to learn from the code, I came across the line

cd_size = pd.GetNumberOfArrays()
.

Shouldn't it be cd.GetNumberOfArrays() instead of pd.GetNumberOfArrays()?

@patrickoleary
Copy link
Member

patrickoleary commented Jun 29, 2022 via email

@dasmy
Copy link
Contributor Author

dasmy commented Jun 29, 2022

Shouldn't it be cd.GetNumberOfArrays() instead of pd.GetNumberOfArrays()?

Opps, fixed my typo. Thanks :-)

@finetjul
Copy link
Member

finetjul commented Jun 30, 2022

Hi dasmy, I think you are right. can you please submit a PR ?

@dasmy
Copy link
Contributor Author

dasmy commented Jul 1, 2022

Hi @finetjul,
As I needed some more fixes / additions to the macro (see https://discourse.paraview.org/t/trouble-exporting-scene-to-vtkjs/9829/10 for the full story), I did several additional modifications, see https://github.com/dasmy/export-scene-macro .

Does it make sense to discuss them and create a composite PR or should I for now only address the change mentioned above?

@DavidBerger98
Copy link
Contributor

Hi @dasmy,

Thank you for opening this issue and suggested changes.

You can create one PR with your changes from the discussion and the fixed typo. Linking the discourse thread and this issue in the PR should be enough.

I am pinging @jourdain in case he wants to add anything to the discussion.

Best,

David

@jourdain
Copy link
Collaborator

jourdain commented Jul 5, 2022

I'm good pushing that macro forward. In general. It is a "deprecated path" now that we have a more mainstream C++ one. But it is definitely an easier path for the community to contribute and extend it to better fit their needs.

dasmy added a commit to dasmy/vtk-js that referenced this issue Jul 6, 2022
* fix import errors
* fix Python3 compatibility
* fix typo (pd.GetNumberOfArrays() instead of cd.GetNumberOfArrays()), fixes Kitware#2499
dasmy added a commit to dasmy/vtk-js that referenced this issue Jul 6, 2022
* fix import errors
* fix Python3 compatibility
* fix typo (pd.GetNumberOfArrays() instead of cd.GetNumberOfArrays()), fixes Kitware#2499
dasmy added a commit to dasmy/vtk-js that referenced this issue Jul 6, 2022
* fix import errors
* fix Python3 compatibility
* fix typo (pd.GetNumberOfArrays() instead of cd.GetNumberOfArrays()), fixes Kitware#2499
dasmy added a commit to dasmy/vtk-js that referenced this issue Jul 6, 2022
* fix import errors
* fix Python3 compatibility
* fix typo (pd.GetNumberOfArrays() instead of cd.GetNumberOfArrays()), fixes Kitware#2499
@github-actions
Copy link

🎉 This issue has been resolved in version 25.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Automated label label Jul 14, 2022
JiayiXuu pushed a commit that referenced this issue Jul 21, 2022
* fix import errors
* fix Python3 compatibility
* fix typo (pd.GetNumberOfArrays() instead of cd.GetNumberOfArrays()), fixes #2499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Automated label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants