Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug: microsecond must be in 0..999999 #1655

Closed
hujiaxing opened this issue Jan 22, 2021 · 15 comments · Fixed by #1683
Closed

fix bug: microsecond must be in 0..999999 #1655

hujiaxing opened this issue Jan 22, 2021 · 15 comments · Fixed by #1683

Comments

@hujiaxing
Copy link

Note that issues in this repository are only for bugs or feature requests in the pywin32.

If you need support or help using this package, please follow these instructions - support or help requests will be closed without comment.

For all bugs, please provide the following information.

  • Expected behavior and actual behavior.

  • Steps to reproduce the problem.

  • Version of Python and pywin32

@hujiaxing
Copy link
Author

in PyTime.cpp

PyObject *PyWinObject_FromDATE(DATE t)
{

#ifdef PYWIN_HAVE_DATETIME_CAPI
// via https://www.codeproject.com/Articles/17576/SystemTime-to-VariantTime-with-Milliseconds
// (in particular, see the comments)
double fraction = t - (int)t; // extracts the fraction part
double hours = (fraction - (int)fraction) * 24.0;
double minutes = (hours - (int)hours) * 60.0;
double seconds = round((minutes - (int)minutes) * 60.0, 4);
double milliseconds = round((seconds - (int)seconds) * 1000.0, 0);
// the following is my fix
// if seconds has decimal part greater than 0.9995, it will be converted to 1000 milliseconds,
// and an exception will be caused: "microsecond must be in 0..999999",
// so set milliseconds to 999
if ((seconds - (int)seconds) >= 0.9995) {
milliseconds = 999.0;
}
// assert(milliseconds>=0.0 && milliseconds<=999.0);

    // Strip off the msec part of time
    double TimeWithoutMsecs = t - (ONETHOUSANDMILLISECONDS / 1000.0 * milliseconds);

    // Let the OS translate the variant date/time
    SYSTEMTIME st;
    if (!VariantTimeToSystemTime(TimeWithoutMsecs, &st)) {
        return PyWin_SetAPIError("VariantTimeToSystemTime");
    }
    if (milliseconds > 0.0) {
        // add the msec part to the systemtime object
        st.wMilliseconds = (WORD)milliseconds;
    }
    return PyWinObject_FromSYSTEMTIME(st);

#endif // PYWIN_HAVE_DATETIME_CAPI

#ifndef NO_PYWINTYPES_TIME
return new PyTime(t);
#endif
}

Is that a properly fix?

@mhammond
Copy link
Owner

Do you have a test case for the issue being fixed?

@hujiaxing
Copy link
Author

hujiaxing commented Jan 27, 2021

Do you have a test case for the issue being fixed?

I used pywin32 to access outlook and sometimes exception thrown by pywin32.
It is critical value problem, so I just set milliseconds to 999 and program work fine.

@hujiaxing
Copy link
Author

@mhammond

In PyTime.cpp

static double round(double Value, int Digits)
{
assert(Digits >= -4 && Digits <= 4);
int Idx = Digits + 4;
double v[] = {1e-4, 1e-3, 1e-2, 1e-1, 1, 10, 1e2, 1e3, 1e4};
return floor(Value * v[Idx] + 0.5) / (v[Idx]);
}

double milliseconds = round((seconds - (int)seconds) * 1000.0, 0); =>

double value = seconds - (int)seconds; // >= 0.9995
value = value * 1000; // >= 999.5
round(value, 0); =>

  return floor(value + 0.5) // will return 1000.0

so milliseconds will cause an exception

@mhammond
Copy link
Owner

FWIW, I spent some time trying to reproduce this with the test COM objects and failed. I'll have another go soon, but I'm reluctant to check in a speculative fix without any certainty around whether it actually fixes the problem.

@hujiaxing
Copy link
Author

@hujiaxing
Copy link
Author

Also this issue: #1466 Some date values in Excel cause ValueError: microsecond must be in 0..999999

@hujiaxing
Copy link
Author

@mhammond any suggestion?

@mhammond
Copy link
Owner

@mhammond any suggestion?

The links above are other reports of the same bug, which I don't doubt exists. However, they don't seem to get me any closer to a repro.

@hujiaxing
Copy link
Author

My fix is not good, I know that
I think there should be another patch for this bug
This bug has been exist for a long time
@mhammond

@mhammond
Copy link
Owner

My fix is not good,

It might be good!

This bug has been exist for a long time

Yeah - which is why I'd rather spend a little more time trying to get a repro rather than making a speculative fix. A tweak to the c++ test server that manages to find a float that causes this problem should be possible, but I failed to find it in my first attempt.

@hujiaxing
Copy link
Author

@mhammond I construct a double date with milliseconds: 18712.308206013888

@bardal
Copy link

bardal commented May 5, 2021

Is there a workaround? I am getting datetimes from Outlook (ReceivedTime on a Message). I don't need microseconds so would happily zero them out ahead of referencing the ReceivedTime which generates the ValueError Exception. Is that possible?

@bardal
Copy link

bardal commented May 6, 2021

Alternatively, do you think this bugfix will be released in the near future?

@mhammond
Copy link
Owner

mhammond commented May 7, 2021

I just merged that PR, so it will be in the next release, but I can't predict when that might be.

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 a pull request may close this issue.

3 participants