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

Consider making BrokenObjectLinkError a warning #99

Open
MicahGale opened this issue Aug 5, 2022 · 4 comments
Open

Consider making BrokenObjectLinkError a warning #99

MicahGale opened this issue Aug 5, 2022 · 4 comments
Labels
feature request An issue that improves the user interface.

Comments

@MicahGale
Copy link
Collaborator

In GitLab by @tjlaboss on Aug 5, 2022, 16:49

MontePy is helping me to move some specimens between models. I've made the following change on my local branch.

Original: cells.py#L80

        for cell in self:
            cell.update_pointers(cells, materials, surfaces)

Local:

        errs = []
        for cell in self:
            try:
                cell.update_pointers(cells, materials, surfaces)
            except BrokenObjectLinkError as e:
                errs.append(str(e))
        warn(f"There were {len(errs)} broken object errors:\n\t" + "\n\t".join(errs))

This is helpful for learning about all of the missing surfaces at once instead of one at a time. The ones not caused by Issue #56 can then be added to the MCNP_Problem instance since it has not raised a fatal error.

Perhaps this would be a nice feature on the main codebase.

@MicahGale
Copy link
Collaborator Author

I still think this should be a fatal error, but yeah having an accumulator might be nice. Maybe we catch and append to the error, and at the very end raise it? This could be easy to solve with recursion.

Should we do it? Make this a recursive function call?

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Aug 6, 2022, 07:32

If it is a warning, than it can be caught and turned into an error if desired: https://docs.python.org/3/library/warnings.html#the-warnings-filter

Perhaps one can have an option_context, with the default being that (some?) warnings are errors: https://pandas.pydata.org/docs/user_guide/options.html

The accumulator will be nice. I think that's something that will be useful to everybody. Is there any advantage to making it recursive instead of just for-looping?

@MicahGale
Copy link
Collaborator Author

With recursion it'd be easy to keep adding to the error before finally raising it without a catch. Basically if you do a depth-first search this should work recursively:

error = None
try:
   recursive_call()
except ExceptionFoo as e:
   error = e

do_stuff
if bad_stuff:
   if error:
       error.append
   else:
      error = ....
   raise error

@MicahGale
Copy link
Collaborator Author

Probably not necessary, but would be fun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request An issue that improves the user interface.
Projects
None yet
Development

No branches or pull requests

1 participant