Skip to content

Conversation

@matthiasdiener
Copy link
Collaborator

@matthiasdiener matthiasdiener commented Nov 15, 2022

Draft because:

@matthiasdiener matthiasdiener self-assigned this Nov 15, 2022
except CycleError as err:
raise PartitionInducedCycleError(err)
# type-ignore-reason: Needs a pytools release with
# https://github.com/inducer/pytools/pull/158 to pass mypy.
Copy link
Owner

Choose a reason for hiding this comment

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

I can do that. Remind me tomorrow.

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I removed the ignore in 1c7969f

raise PartitionInducedCycleError(err)
# type-ignore-reason: Needs a pytools release with
# https://github.com/inducer/pytools/pull/158 to pass mypy.
raise PartitionInducedCycleError(err) # type: ignore[no-untyped-call]
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe unpack the cycle rather than repackaging the whole error.

Copy link
Collaborator Author

@matthiasdiener matthiasdiener Nov 21, 2022

Choose a reason for hiding this comment

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

Is a56643a what you had in mind?

@matthiasdiener matthiasdiener marked this pull request as ready for review November 21, 2022 23:23
@inducer
Copy link
Owner

inducer commented Nov 27, 2022

This has conflicts now, sorry.

@matthiasdiener
Copy link
Collaborator Author

This has conflicts now, sorry.

I think they have been fixed in 372b2e5. This is ready for another review.

@inducer inducer marked this pull request as draft December 12, 2022 16:15
except CycleError:
raise PartitionInducedCycleError
except CycleError as err:
raise PartitionInducedCycleError(err.node)
Copy link
Owner

Choose a reason for hiding this comment

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

Based on inducer/pytools#159, this should probably be err.path.

@inducer
Copy link
Owner

inducer commented Dec 12, 2022

Converted to draft for the time being, with a to-do list in the description.

@inducer
Copy link
Owner

inducer commented May 17, 2023

Superseded by #434 .

@inducer inducer closed this May 17, 2023
@inducer inducer deleted the pass-cycle branch May 17, 2023 22:33
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