Skip to content

Conversation

@mattwthompson
Copy link
Collaborator

This is a short-term band-aid for #247 - implementing them will take more than a trivial amount of time so we should at least error out instead of silently ignoring entire forces in user's systems.

@ijpulidos ijpulidos added this to the 0.11.3 milestone Jul 11, 2023
@ijpulidos ijpulidos self-requested a review July 11, 2023 14:47
@mikemhenry
Copy link
Collaborator

@mattwthompson @ijpulidos I closed a bunch of PRs that I think #290 took care of, is this one still needed?

Comment on lines 1046 to 1048
raise Exception(f"Two instances of force {force_name} appear in System")
if force_name not in supported_forces:
raise Exception(f"Custom forces not supported. Found force of type {force_name}.")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should probably add I don't think this is the right exception to raise, I'm just not sure which it should be

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eh, this package doesn't define many exceptions, but I made a new one

@mikemhenry
Copy link
Collaborator

Not sure why this PR isn't running, @mattwthompson can you remake it when you have the chance?
image

@mattwthompson
Copy link
Collaborator Author

I'm pretty sure #247 is unaddressed, it's not directly related to the other PRs we've had opened.

I'm not sure why I opened this on my fork, moving it to upstream now

@mikemhenry
Copy link
Collaborator

Ah and yes it was a fork thing and not a permissions thing, oops

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