Support dates for exclude-newer tool setting#8553
Support dates for exclude-newer tool setting#8553manzt wants to merge 1 commit intoastral-sh:mainfrom
exclude-newer tool setting#8553Conversation
91f0be2 to
f5b3502
Compare
exclude-newer tool setting
f5b3502 to
2e795ff
Compare
|
What timezone does this use? UTC? I'm not sure if this is quite the same, but we discussed something like this in #6562 and decided against the change. |
|
The docs say:
So I'm guessing whatever the system's time zone is. |
|
Looking at the .and_then(|date| date.to_zoned(TimeZone::system()))If that's not the intention, than maybe we should change the example in the docs. |
|
Yeah I think we might want to change the docs or, if we want to accept a simple date, use UTC. I think the intent is that we want a stable time in @BurntSushi has the most context on this though. |
Makes total sense. I'm thinking the main "push" for flexibility here (as mentioned in #6562) is convenience (i.e., actually typing out the timestamp is annoying), but not that someone adamantly wants ambiguity. Maybe something to consider is just a convenient high-level API for adding this setting correctly: uv add --timestamp
uv add --script foo.py --timestamp
uv add --timestamp=2024-01-01 General idea is that you could have something conveniently resolve the users local time to something correct, keeping strictness with the target. (Maybe EDIT: another idea uv init --with-timestamp --script foo.py |
|
Ahh, seeing you already suggested a command in #6562... just catching up |
|
Yeah I think this kind of needs to wait for a |
|
Yeah, I basically think that we probably should not support writing unadorned dates in a persistent configuration that is meant to be shared among folks that may be in different time zones. It just makes the date ambiguous. Even if we defaulted to UTC, it's still not immediately obvious to someone reading the config file that it would be UTC. And it seems much more natural to assume it isn't UTC but actually local, which would lead to folks reading the date incorrectly in some cases. I think a hypothetical There are other options for writing timestamps to config files that are maybe a little more user friendly. For example, we could support |
|
Sounds like we should update the documentation to clarify this. |
Sounds good to me! Not something I'd considered and appreciate the thoughtfulness and explanation by you both. |
|
Until a built-in command exists, I’ve added uv init --script foo.py
uv add --script foo.py polars
uvx juv stamp foo.py --date=2021-07-07 |
…9135) <!-- Thank you for contributing to uv! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Follow up to #8553 Clarifies that the `exclude-newer` setting must be a full timestamp and not a date. <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan N/A <!-- How was it tested? -->
Summary
hey! I was poking around with the tool support for exclude newer in juv (think this would be awesome to use to make Jupyter notebooks more standalone/reproducible) and ran into an issue.
The settings documentation for
exclude-newersays that it supports both dates and timestamp.However, with uv v0.4.26:
I believe this is because the derive'd Deserialize from jiff only supports timestamps and doesn't use the more flexible FromStr implemention implemented for
ExlcudeNewerTest Plan
Added a test with a date string