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

option to make end offsets inclusive #271

Open
dave-doty opened this issue Aug 23, 2023 · 0 comments
Open

option to make end offsets inclusive #271

dave-doty opened this issue Aug 23, 2023 · 0 comments
Labels
challenging high priority Something cruicial to get working soon. invalid This doesn't seem right

Comments

@dave-doty
Copy link
Member

dave-doty commented Aug 23, 2023

It's clear to me now that making the end offset of each Domain exclusive was a mistake. It leads to situations that are hard to reason about, for instance to draw a strand backwards you have to think about jumping to offsets that are one greater than the one you really want, e.g.,

design.draw_strand(0, 16).move(-4).cross(0, 11).move(-4)

Creates this design:

image

Note the awkwardness that the two offsets specified, 16 and 11, are not part of the strand at all. These seems less strange when specifying domains by creating explicit Domain objects, but here it's strange that calling draw_strand or cross with an offset, you don't know whether that offset will actually be part of the strand or not, until the next call to move or to that indicates whether the domain will be forward or reverse.

Add a field to Design called end_offsets_inclusive, with default value False, that controls this. If set to True then end offsets will be inclusive.

Some unit tests will need to be added to ensure the new interpretation works.

We can slowly deprecate first by changing the default value to True so that, after we make that change, users will need to explicitly specify a value of False in the Design constructor to make the old behavior appear. If it seems necessary we can eventually remove the field altogether and solely use inclusive offsets.

@dave-doty dave-doty added high priority Something cruicial to get working soon. invalid This doesn't seem right labels Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenging high priority Something cruicial to get working soon. invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant