BUG: Timestamp fails when fold is passed with positional args
State: closed

- [ ] closes #52117
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Timezones
description: Timezone data dtype
name: Timestamp
description: pd.Timestamp and associated methods

Comments:
Now I see what happened. The error check treats an `int` in `ts_input` as time since epoch, so it raises an error. In `1.5.3` we had a timezone-aware datetime by that point, but now we have an int. Let's see how we can fix it.
As I mentioned in #52117 , we should also fix the fact that when we use positional arguments currently, the year gets put into `ts_input` and the rest of arguments are shifted by 1. This is very non-intuitive and is a problem for maintaining the code.
@mroeschke Would you mind taking a look?

~~The bugfix works by checking whether we have passed positional args or not. Currently, that means that the year ends up in ts_input. My suggestion would be to merge this safe fix and then figure out the best way to make sure that the mess with variable names and their content gets fixed.~~

Scratch that, this way of fixing the bug is stupid. Let's try fixing the core issue instead, so we don't need two PRs where one will suffice.
We can also attempt to deal with the overarching problem in this PR. If we make sure that `ts_input` is set to `_no_input` for positional args, that would also be a fix. I'd rather do it in a separate PR though, because I'm not sure yet, what the reason for the changes was.
Is the long-term solution here to use `*args, **kwargs` so we can disambiguate what was passed positionally vs by keyword?
@jbrockmendel @mroeschke 
As a medium-term solution to fix both #52117 and the bug I discovered and outlined [here](https://github.com/pandas-dev/pandas/pull/52221#discussion_r1150997581), I propose we wrap the Timestamp class with a decorator to fix the `args` contents before calling `__new__`.

I've pushed a working solution, please take a look. I ran asv benchmarks on it, and there is a performance hit:
```
     <main>           <gh-51117-positional-fold-bug>
+         264±5ns          379±2ns     1.43  tslibs.timestamp.TimestampConstruction.time_from_pd_timestamp
+     1.07±0.02μs      1.26±0.01μs     1.17  tslibs.timestamp.TimestampConstruction.time_fromordinal
+        836±10ns         952±10ns     1.14  tslibs.timestamp.TimestampConstruction.time_from_datetime_aware
+     1.05±0.01μs      1.17±0.01μs     1.12  tslibs.timestamp.TimestampConstruction.time_parse_iso8601_no_tz
+         886±4ns          989±4ns     1.12  tslibs.timestamp.TimestampConstruction.time_from_npdatetime64
```
If I did something dumb performance-wise, please tell me. I think, we can ignore `time_from_pd_timestamp`, because that's a super-fast shortcut, so the overhead there is very noticeable for asv but will be negligible for users. For more realistic cases, the decorator looks to slow down construction by 10-20%.

In my opinion, fixing this by restoring proper `args` is worth the performance hit, but that's mostly me not liking that we have values not matching argument names inside a core class and making the code vulnerable to bugs.
Great, the code passed the checks on the second try, now ready for a review.
> Is the long-term solution here to use *args, **kwargs so we can disambiguate what was passed positionally vs by keyword?

I would be okay with going with this solution. The decorator method is clever but I think refactoring using args, kwargs is a little more clear
> I would be okay with going with this solution. The decorator method is clever but I think refactoring using args, kwargs is a little more clear

Long-term I would also go with args and kwargs. The decorator, I think, is okay for now, but it is still a kludge.

As much as I try to avoid relying on args and kwargs (because of their propensity toward spaghetti code and all the documentation and sanity checking that's always needed to make sure that everything is safe, clean and readable), I see no clean way to fulfill our promise of supporting all the datetime construction conventions without moving to *args and *kwargs.

My suggestion is to accept this fix, and then make a separate ticket to refactor the class to use `*args, **kwargs` (I'll be able to take the ticket, but I anticipate it taking a bit to implement and document), but I'll go with what you two think. 
why can't u do this inside new itself

this is quite a kludge and the perf hit is not nothing
> why can't u do this inside new itself
> 
> this is quite a kludge and the perf hit is not nothing

Hello, @jreback , happy to talk to you again.

We need to detect whether an argument was passed via positional args or kwargs, or we run into the bug that @mroeschke pointed out [here](https://github.com/pandas-dev/pandas/pull/52221#discussion_r1149650989):
```Timestamp(2023, 1, 1, minute=1)```
Do we shift here or not? A better illustrative example on `main` might be this: 
```python
import pandas as pd

ts1 = pd.Timestamp(2023, 3, 30, 10, None, None, 30)
ts2 = pd.Timestamp(2023, 3, 30, 10, second=30)

print(ts1)
print(ts2)
> 2023-03-30 10:00:00.000030
> 2023-03-30 10:00:00.000030
```
On `main` these give the same answer, because the variable content ends up the same.

I was not able to find a way to determine from within a function, whether an argument was passed positionally or via kwargs. The only ways seem to be via a decorator or by refactoring the function to take `*args, **kwargs` and then add the logic to parse them into variables.

If I've missed something, please tell me. I'd love to get the right variable contents from within `__new__` without kludges.
not what i mean

we have named args here already 
all you would need to see if they are shifted right?
In order to see whether an argument needs to be shifted to avoid bugs, we need to determine if it was passed positionally or by kwarg. That's because the variable state at the start of `__new__` is the same for the following two cases, but in the first one `microsecond` ended up in `second` and needs to be shifted, and in the second case `second` is where it belongs, because it was passed through kwargs:

```python
pd.Timestamp(2023, 3, 30, 10, None, None, 30)
pd.Timestamp(2023, 3, 30, 10, second=30)
```

@jreback , what am I missing?
Closing as a different approach and a broader bug definition is needed.