Uh oh!
There was an error while loading. Please reload this page.
Fix use_dual type hints - #1612
Conversation
The public grid-loading APIs document and implement use_dual as a boolean, but their annotations also accepted None. Align the public and internal signatures with the actual contract and add focused regression coverage.
Sevans711
commented
Jul 23, 2026
Thank you for these contributions, @Sanjays2402! This looks like a clean fix to the None type hints on the use_dual kwarg, and one good way to test things. I have a few thoughts/notes before we get this merged into main:
Also, out of curiosity, @Sanjays2402 what led you to uxarray? It is always helpful to hear how people find out about the project! |
cmdupuis3
commented
Jul 24, 2026
Yeah, I've been noticing this too, seems like an anti-pattern to allow both |
Sanjays2402
commented
Jul 25, 2026
Agreed on the scope, "fixes part of #1593" is the accurate framing, On the test: I'd lean (B). It's spot-check by nature and I share your concern about setting an expectation that every kwarg gets a hand-written type-hint test, that gets unmaintainable fast and the test does key off hint spelling. Happy to add a comment saying as much. If the team would rather go (C) I won't argue, the signature fix stands on its own without it. Say which way you want it and I'll push. |
Sevans711
commented
Jul 29, 2026
Thank you again for your contributions here, @Sanjays2402. Letting you know, there is some internal discussion happening with other uxarray devs about how people want to proceed in this case. Will provide an update here with decision / reasoning, once that discussion concludes! |
Sanjays2402
commented
Aug 15, 2026
Understood — I will leave this open while the team decides between the spot-check comment, dropping the test, or a broader type-hint check. The four signature fixes are intentionally scoped to this part of #1593; I will wait for that decision before changing the branch. |
Sevans711
commented
Aug 17, 2026
Sounds good, also thank you for sending a reply here! This was a good reminder for me. I wanted to report back sooner but the discussion didn't lead to a final decision on how to move forward yet. Rather, it seems like the plan will be for @dylannelson to take point on how to address the original issue fully throughout the codebase, and to report back here about how to proceed with this PR after getting a chance to look into it further. Hopefully sometime soon, but it might also be a somewhat lower priority because it is isn't causing any crashes, just confusing type hints. |
Fixes part of #1593 but does not fully close it.
Overview
use_dualtoboolacross the public grid-loading APIs andGrid.from_dataset, matching the documented and implemented contract.PR Checklist
General
Testing
Documentation
_) and have been added todocs/internal_api/index.rstdocs/user_api/index.rstExamples
docs/examples/folderdocs/examples.rsttoctreedocs/gallery.ymlwith appropriate thumbnail photo indocs/_static/thumbnails/