Skip to content

fix: Raise AttributeError for non-existing meshing objects after switch_to solver. #3949

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

Merged
merged 18 commits into from
Apr 25, 2025

Conversation

prmukherj
Copy link
Collaborator

@prmukherj prmukherj commented Apr 23, 2025

Raise AttributeError for non-existing meshing objects after switch_to solver.

Earlier instead of raising an Attribute error (general python behaviour), it was returning None. This has been corrected now.

There is an "is_active" method associated with session objects to check it's state.

@github-actions github-actions bot added the bug Issue, problem or error in PyFluent label Apr 23, 2025
@seanpearsonuk
Copy link
Collaborator

Hi @prmukherj,

Thanks for the improvements here — I agree that raising AttributeError on access to a switched meshing session is more idiomatic and aligns better with general Python expectations around invalid states.

That said, I wanted to highlight a few related areas that may warrant attention to ensure consistent and intuitive behavior:

  1. dir(<mesh_session>) still raises a low-level exception, both before and after this change, due to access to internal attributes during __dir__ resolution. This could be surprising and uninformative to users. We may need to explicitly handle the switched state in __dir__() and return a helpful message or an empty list.

  2. help(<mesh_session>) now presents stale info after the switch (which is a regression). It gives the impression the interface is still usable. This could mislead users who turn to help() for post-switch diagnostics. If there’s a way to influence help() output (e.g., overriding __doc__ or __getattribute__ with a custom response), we might consider doing so for clarity.

  3. No public API to check if the session is active. At the moment, the only way to determine whether the meshing session is still valid is to access the private _switched attribute. It would be helpful to expose a method like is_active() or has_been_switched() to allow users to make this check in a clean and forward-compatible way.

Happy to discuss any of these further — let me know what you think.

@prmukherj
Copy link
Collaborator Author

Hi @prmukherj,

Thanks for the improvements here — I agree that raising AttributeError on access to a switched meshing session is more idiomatic and aligns better with general Python expectations around invalid states.

That said, I wanted to highlight a few related areas that may warrant attention to ensure consistent and intuitive behavior:

  1. dir(<mesh_session>) still raises a low-level exception, both before and after this change, due to access to internal attributes during __dir__ resolution. This could be surprising and uninformative to users. We may need to explicitly handle the switched state in __dir__() and return a helpful message or an empty list.
  2. help(<mesh_session>) now presents stale info after the switch (which is a regression). It gives the impression the interface is still usable. This could mislead users who turn to help() for post-switch diagnostics. If there’s a way to influence help() output (e.g., overriding __doc__ or __getattribute__ with a custom response), we might consider doing so for clarity.
  3. No public API to check if the session is active. At the moment, the only way to determine whether the meshing session is still valid is to access the private _switched attribute. It would be helpful to expose a method like is_active() or has_been_switched() to allow users to make this check in a clean and forward-compatible way.

Happy to discuss any of these further — let me know what you think.

Yes @seanpearsonuk, I agree with this proposal. These changes are necessary. Others please put in your views as well.
Thank you.

mkundu1

This comment was marked as resolved.

@mkundu1
Copy link
Contributor

mkundu1 commented Apr 23, 2025

Can you please explain the *_wo_exit changes in tests?

@prmukherj
Copy link
Collaborator Author

Can you please explain the *_wo_exit changes in tests?

Yes, these tests have to be exited manually as they are meshing sessions switching to solver in the test body.

Copy link
Contributor

@mkundu1 mkundu1 left a comment

Choose a reason for hiding this comment

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

The private _switched attribute is probably no longer required now.

@prmukherj
Copy link
Collaborator Author

The private _switched attribute is probably no longer required now.

Agreed, Removed now. Will need to update PyConsole accordingly in the next release.

@seanpearsonuk
Copy link
Collaborator

seanpearsonuk commented Apr 25, 2025

@prmukherj Is there a test for help(<meshing_session>)?

@prmukherj
Copy link
Collaborator Author

@prmukherj Is there a test for help(<meshing_session>)?

No, the help behavior was not updated yet. Will update it as part of this PR only then. Converting it to draft for further testing.

@prmukherj prmukherj marked this pull request as draft April 25, 2025 11:03
@prmukherj prmukherj marked this pull request as ready for review April 25, 2025 13:47
@prmukherj prmukherj merged commit ea4bea5 into main Apr 25, 2025
31 of 32 checks passed
@prmukherj prmukherj deleted the fix/raise_error_for_non_existing_meshing_attrs branch April 25, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue, problem or error in PyFluent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants