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

Mostieri/particle traces #338

Merged
merged 7 commits into from
Nov 21, 2023
Merged

Mostieri/particle traces #338

merged 7 commits into from
Nov 21, 2023

Conversation

mariostieriansys
Copy link
Collaborator

No description provided.

@mariostieriansys
Copy link
Collaborator Author

@randallfrank @kecolburn at the moment this PR is in draft mode so that we can discuss about the API before merging it.

The way I have designed it is that the user sees exposed only one function, create_particle_trace.

I have used Randy suggestion of creating subclasses of ens_emitterobj, which simplify the handling of the creation of the objects. However these classes really work only in EnSight, i.e. if in ensight you call ensight.utils.parts._EnSEmitterPoint(ensight, point1=[1,1,1]) will actually create a valid emitter.
While the class is also defined in PyEnSight, I am not using it there because it doesn't work. The issues is that in PyEnSight ens_emitterobj is an ENSOBJ, in EnSight it isn't. I have tried to workaround with _convert_ctor() in session.cmd() this issue, but I have not been able so far to convert what I receive back from EnSight to a PyEnSight object. Also, even if I would, assigning directly the emitters would need also the conversion of the PyEnSight emitters to EnSight.
So at the end I decided to make the classes private, being used in background directly in EnSight, and if I am in a PyEnSight environment I will just build them directly via session.cmd(). This approach works, and the user won't have to create emitters by themselves but they can just use one API.

Technicalities apart, the PR isn't complete for merge:

  1. I have considered the pathline case, but haven't tested it yet. Also I have to add an input with the timesteps I want to use for a pathline
  2. There's no input for the pathline visualization. I believe I could also return the particle trace and let the user change its attributes, but probably allowing it from the API would be better

Any suggestion would be very welcome

@mariostieriansys mariostieriansys marked this pull request as ready for review November 17, 2023 16:43
@codecov-commenter
Copy link

codecov-commenter commented Nov 17, 2023

Codecov Report

Attention: 165 lines in your changes are missing coverage. Please review.

Comparison is base (d16685f) 79.92% compared to head (85f2145) 75.40%.

Files Patch % Lines
src/ansys/pyensight/core/utils/parts.py 17.91% 165 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
- Coverage   79.92%   75.40%   -4.52%     
==========================================
  Files          18       18              
  Lines        2570     2769     +199     
  Branches      454      495      +41     
==========================================
+ Hits         2054     2088      +34     
- Misses        357      522     +165     
  Partials      159      159              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

randallfrank
randallfrank previously approved these changes Nov 17, 2023
src/ansys/pyensight/core/utils/parts.py Outdated Show resolved Hide resolved
@mariostieriansys mariostieriansys merged commit b5d3f55 into main Nov 21, 2023
21 checks passed
@mariostieriansys mariostieriansys deleted the mostieri/particle_traces branch November 21, 2023 11:15
mariostieriansys added a commit that referenced this pull request Nov 21, 2023
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