Skip to content

chore: Enhance FileSession batching reliability and error clarity#5061

Open
prmukherj wants to merge 12 commits intomainfrom
maint/tody_up_file_session
Open

chore: Enhance FileSession batching reliability and error clarity#5061
prmukherj wants to merge 12 commits intomainfrom
maint/tody_up_file_session

Conversation

@prmukherj
Copy link
Copy Markdown
Collaborator

@prmukherj prmukherj commented Apr 9, 2026

Context

FileSession batching behavior had several issues: scalar field requests incorrectly reflected node/boundary semantics that don't apply to file-based sessions, batch responses overwrote prior results when multiple requests targeted the same surface, and public docstrings misled callers about node_value/boundary_value behavior.

Change Summary

  • Scalar option normalization: node_value/boundary_value are normalized to False for FileSession scalar reads (face/element-only), emitting a PyFluentDeprecationWarning with correct stacklevel when non-default values are passed.
  • Consistent enum usage: DataLocation enum used in both tag construction and response aggregation, eliminating fragile integer-to-enum key alignment.
  • _ScalarFieldTransaction defaults: node_value/boundary_value now default to False, accurately reflecting FileSession semantics regardless of instantiation path.
  • Batch response merging: Switched to setdefault-based aggregation so multiple scalar/vector field requests for the same surface merge instead of overwriting.
  • Selective mesh reads: Batch.get_response now respects provide_faces/provide_vertices flags, skipping unnecessary connectivity/vertex reads for surfaces that didn't request them.
  • Docstrings: Updated add_scalar_fields_request and get_scalar_field_data to document that node_value/boundary_value are ignored in FileSession (face-only data) and emit a deprecation warning when set to True.

Rationale

FileSession reads data from files (face/element only); exposing node/boundary options as meaningful controls is incorrect and misleading. Merging rather than overwriting batch results is necessary for correctness when multiple field requests share a surface.

Impact

  • FileSession scalar field batching and direct reads.
  • Callers passing node_value=True or boundary_value=True will now see a deprecation warning.
  • Batch responses for multi-request scenarios will no longer silently drop earlier results.

@prmukherj prmukherj requested a review from Copilot April 9, 2026 07:30
@github-actions github-actions bot added the enhancement Improve any current implemented feature label Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tidies up FileSession batching behavior so that batched field-data requests correctly normalize FileSession-only scalar semantics and merge multiple requests for the same surface without overwriting prior results.

Changes:

  • Added scalar option normalization for FileSession scalar reads (node/boundary options normalized to face/element semantics with a deprecation warning).
  • Updated batch response aggregation to merge multiple scalar/vector fields and surface data per surface via setdefault (instead of overwriting).
  • Added tests to validate scalar request option handling and per-surface merging behavior in batch responses.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tests/test_file_session.py Adds regression tests covering scalar option normalization behavior and per-surface merging in batch responses.
src/ansys/fluent/core/file_session.py Normalizes scalar request options for FileSession, improves batch aggregation merging, and tightens vector field name validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread src/ansys/fluent/core/file_session.py Outdated
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 9, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

prmukherj and others added 5 commits April 9, 2026 13:17
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ansys/fluent/core/file_session.py
Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread src/ansys/fluent/core/file_session.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@prmukherj prmukherj marked this pull request as ready for review April 9, 2026 10:30
Copilot AI review requested due to automatic review settings April 9, 2026 10:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ansys/fluent/core/file_session.py
Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread src/ansys/fluent/core/file_session.py
Comment thread tests/test_file_session.py
@seanpearsonuk
Copy link
Copy Markdown
Collaborator

@prmukherj I notice that Copilot used "tidies up" in its PR summary, but it writes "tidies up FileSession batching behavior," whereas titling the PR chore: Tidy up File Session gives a false impression that the PR does not include any behavioural changes. This becomes a problem when we read the logs later. It would be better to emphasise in the title that this PR is all about fixes.

@prmukherj prmukherj changed the title chore: Tidy up FileSession chore: Enhance FileSession batching reliability and error clarity Apr 10, 2026
Comment thread src/ansys/fluent/core/file_session.py
Co-authored-by: Sean Pearson <93727996+seanpearsonuk@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 10, 2026 09:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ansys/fluent/core/file_session.py
Comment thread src/ansys/fluent/core/file_session.py
Comment thread src/ansys/fluent/core/file_session.py Outdated
Comment thread tests/test_file_session.py
Comment thread src/ansys/fluent/core/file_session.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 10, 2026 09:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ansys/fluent/core/file_session.py
Comment thread src/ansys/fluent/core/file_session.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improve any current implemented feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants