') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix: support negative indices in OrderedSet.__getitem__ by binggao1230 · Pull Request #633 · fgmacedo/python-statemachine · GitHub
Skip to content

fix: support negative indices in OrderedSet.__getitem__ - #633

Merged
fgmacedo merged 2 commits into
fgmacedo:developfrom
binggao1230:fix/orderedset-negative-index
Jun 26, 2026
Merged

fix: support negative indices in OrderedSet.__getitem__#633
fgmacedo merged 2 commits into
fgmacedo:developfrom
binggao1230:fix/orderedset-negative-index

Conversation

@binggao1230

@binggao1230binggao1230 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What

OrderedSet.__getitem__ raised ValueError (from itertools.islice) when called with a negative index, instead of the IndexError callers expect — or, better, returning the element counting from the end, as all Python sequences do.

s=OrderedSet([1, 2, 3])
s[-1] # ValueError: Indices for islice() must be None or an integer: 0 <= x <= sys.maxsize.

Why

itertools.islice rejects negative start/stop values with ValueError. Since __getitem__ is documented and exported as a sequence-style accessor (positive indices already work, positive out-of-range already raises IndexError), negative indices should follow the same contract as Python's built-in sequences.

Fix

Normalise the index before delegating to islice:

  • if index < 0, add len(self) to convert it to the equivalent positive offset
  • if still negative after normalisation, raise IndexError with the original index in the message (not the normalised value)
s[-1] # 3 ✓s[-3] # 1 ✓s[-4] # IndexError: index -4 out of range ✓

Two new unit tests cover the happy path and the out-of-range case. Doctests in orderedset.py are updated with examples of s[-1] and s[-3].

This pull request was prepared with the assistance of AI, under my direction and review.

@binggao1230
fix: support negative indices in OrderedSet.__getitem__
716fd73
Negative index access (e.g. ``s[-1]``) previously raised ``ValueError``
from ``itertools.islice``, which forbids negative start values. This is
surprising because Python sequences uniformly accept negative indices
(``s[-1]`` returns the last element) and the custom ``__getitem__`` on
``OrderedSet`` already raises ``IndexError`` for positive out-of-range
indices.
Normalise the index before passing it to ``islice``: add ``len(self)``
when ``index < 0``, and raise ``IndexError`` (not ``ValueError``) when
the normalised index is still negative. The original index is preserved
in the error message so the reported value matches what the caller passed.
@codecov

codecovBot commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a2457a5) to head (d1f5d9b).

Additional details and impacted files
@@ Coverage Diff @@## develop #633 +/- ##
=========================================
Coverage 100.00% 100.00% =========================================
Files 52 52 Lines 5494 5499 +5 Branches 865 867 +2 =========================================
+ Hits 5494 5499 +5
FlagCoverage Δ
unittests100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@binggao1230

binggao1230 commented Jun 25, 2026

Copy link
Copy Markdown
ContributorAuthor

The previous CI run failed on a timing-sensitive flaky test unrelated to this PR's changes:

`FAILED tests/test_contrib_timeout.py::TestTimeoutComposition::test_timeout_fires_before_slow_invoke[async]`

The assertion `assert 'stuck' in OrderedSet(['loading'])` failed because the 50ms timeout didn't fire within the CI window. The test passes consistently locally. I've pushed a minor fixup commit (removing extra blank lines in the docstring) to re-trigger CI. Could you approve the workflow run so CI can run again?

@fgmacedo

fgmacedo commented Jun 26, 2026

Copy link
Copy Markdown
Owner

Hi @gaoflow, thanks for the careful fix and tests! Verified locally: tests and doctests pass, and the new branches in __getitem__ are fully covered.

@fgmacedo
fgmacedo merged commit 2c1aa0c into fgmacedo:developJun 26, 2026
10 checks passed
@fgmacedofgmacedo self-assigned this Jun 26, 2026
@sonarqubecloud

sonarqubecloudBot commented Jun 26, 2026

Copy link
Copy Markdown

Quality Gate PassedQuality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@fgmacedofgmacedo mentioned this pull request Jun 26, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@fgmacedofgmacedo

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@binggao1230@fgmacedo