Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 24, 2025

Click issue #3069 caused optional parameters to return Sentinel objects instead of None. This was fixed in Click 8.3.1, making the workaround obsolete.

Changes

  • Removed _value_or_none() helper function that checked for Sentinel objects
  • Removed all calls to _value_or_none() (11 occurrences across insert_upsert_implementation(), memory(), and _maybe_register_functions())
  • Updated Click dependency to >=8.3.1 in pyproject.toml

The removed workaround was converting parameters like this:

# Before
def insert_upsert_implementation(..., convert, delimiter, encoding, ...):
    convert = _value_or_none(convert)      # Check if Sentinel, return None
    delimiter = _value_or_none(delimiter)  # Check if Sentinel, return None
    encoding = _value_or_none(encoding)    # Check if Sentinel, return None
    # ...

# After  
def insert_upsert_implementation(..., convert, delimiter, encoding, ...):
    # Click 8.3.1+ returns None directly for unset optional parameters
    # ...
Original prompt

This section details on the original issue you should resolve

<issue_title>Remove these Sentinel hacks when Click 8.3.1 ships</issue_title>
<issue_description>This commit: 1361ed5

Has a workaround for this issue:

The fix for that is set to go out in Click 8.3.1 - release issue here:

Once that is released, remove the nasty hacks.

</issue_description>

Comments on the Issue (you are @copilot in this section)

@simonw This looks like it may be a clean revert:
git revert 1361ed571122ffd4dd8c3f1fdea52a1423422c04</body></comment_new>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: simonw <9599+simonw@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove Sentinel hacks after Click 8.3.1 release Remove Sentinel workaround for Click 8.3.1+ Nov 24, 2025
Copilot AI requested a review from simonw November 24, 2025 08:49
@simonw simonw marked this pull request as ready for review November 24, 2025 17:02
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.33%. Comparing base (ae58faf) to head (20fa91a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #685      +/-   ##
==========================================
+ Coverage   95.32%   95.33%   +0.01%     
==========================================
  Files           8        8              
  Lines        3014     3001      -13     
==========================================
- Hits         2873     2861      -12     
+ Misses        141      140       -1     

☔ View full report in Codecov by Sentry.
📢 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.

@simonw simonw merged commit e66e82f into main Nov 24, 2025
134 checks passed
@simonw simonw deleted the copilot/remove-sentinel-hacks branch November 24, 2025 17:11
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.

Remove these Sentinel hacks when Click 8.3.1 ships

2 participants