Skip to content

[SYNPY-1883] convert date to epoch ms - #1429

Open
danlu1 wants to merge 13 commits into
developfrom
SYNPY-1883-convert-date-to-epoch-ms
Open

[SYNPY-1883] convert date to epoch ms#1429
danlu1 wants to merge 13 commits into
developfrom
SYNPY-1883-convert-date-to-epoch-ms

Conversation

@danlu1

@danlu1 danlu1 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem:

Synapse DATE columns store an exact moment in time as an integer, milliseconds, since the Unix epoch. But the client only handled that conversion in one direction:

  1. Read side ✅ — _convert_df_date_cols_to_datetime converted epoch integers back into datetime objects when querying.
  2. Write side ❌ — nothing converted datetime values into epoch integers before upload. A datetime object (or a formatted date string in a CSV) was passed straight through to the API, which rejected it with a 400 error.

This hit every input type: pandas DataFrames, dict and csv files — and the gap was hidden because the integration test exercising itmanually pre-converted dates with utils.to_unix_epoch_time before calling store_rows, instead of relying on the client to do it.

Also, there is unresolved timezone ambiguity. Miss document to explain how different datetime and date, including tz-aware, naive, mixed timezone datetime and plain date, are converted in the client. Also, csv file columns with mixed UTC offsets had no normalization rule either.

Solution:

The fix adds the missing write-side conversion, plumbed into every path that sends rows to Synapse.

New conversion function _convert_df_date_cols_to_epoch_time auto-detects datetime/date columns and converts them via to_unix_epoch_time and is called in DataFrame, csv uploads and upserts.

New CSV parameters date_columns/date_format in store_rows_async since raw CSVs have no dtype info, the client needs to be told which columns are dates and how to parse them.
_parse_df_date_cols_to_datetime is introduced to parses the strings, _convert_csv_date_cols_to_epoch_time converts and writes a temp CSV that's actually uploadedin finally.

Explicit timezone policy: tz-aware datetimes convert to UTC exactly (recommended); naive datetimes/dates use the upload machine's current local timezone (documented DST caveat in to_unix_epoch_time's docstring; CSV columns with mixed offsets are normalized to UTC.

New tutorial section table.md covers tz-aware vs. naive datetimes, plain dates, CSV date columns, and querying back with convert_to_datetime=True.

Testing:

Three new integration tests in test_table_async.py upload DataFrames/CSVs with tz-aware, DST-crossing, naive, and date-object columns without any manual pre-conversion, verifying the exact epoch values land correctly .

Unit tests for newly added functions and store_row_async have been added.

@danlu1
danlu1 requested a review from a team as a code owner July 21, 2026 19:29
@danlu1
danlu1 requested review from a team and Copilot and removed request for a team July 21, 2026 19:29
@danlu1 danlu1 changed the title Synpy 1883 convert date to epoch ms [SYNPY-1883] convert date to epoch ms Jul 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the missing write-side handling for Synapse DATE columns by converting Python/pandas date-like values into epoch milliseconds before rows are uploaded (DataFrame, dict, upsert, and CSV paths), and documents the timezone policy for those conversions.

Changes:

  • Add _convert_df_date_cols_to_epoch_time (and CSV helpers) and wire the conversion into store/upsert flows.
  • Add date_columns / date_format parameters for CSV uploads so formatted date strings can be parsed then converted before upload.
  • Expand docs and tests (unit + integration) to cover tz-aware, naive, DST-crossing, and datetime.date inputs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/unit/synapseclient/mixins/unit_test_table_components.py Adds unit tests covering DataFrame/CSV date parsing + epoch conversion and store_rows plumbing.
tests/integration/synapseclient/models/async/test_table_async.py Adds integration coverage for end-to-end DATE storage from DataFrames and CSVs (tz-aware/naive/date objects).
synapseclient/models/table.py Public API doc updates + new CSV parameters (date_columns, date_format) for store_rows.
synapseclient/models/mixins/table_components.py Implements and integrates the conversion/parsing helpers into store/upsert logic.
synapseclient/core/utils.py Clarifies and documents to_unix_epoch_time conversion rules (tz-aware vs naive vs date).
docs/tutorials/python/table.md Adds a tutorial section explaining DATE semantics and examples for storing/querying datetimes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread synapseclient/models/mixins/table_components.py
Comment thread synapseclient/models/mixins/table_components.py
Comment thread docs/tutorials/python/table.md
Comment thread docs/tutorials/python/table.md Outdated
Comment thread docs/tutorials/python/table.md
danlu1 and others added 9 commits July 21, 2026 13:38
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Updated code block formatting for Python example in table.md.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread docs/tutorials/python/table.md
Comment thread docs/tutorials/python/table.md
Comment on lines 2410 to 2413
async def upsert_rows_async(
self,
values: Union[str, Dict[str, Any], DATA_FRAME_TYPE],
primary_keys: List[str],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this upsert method also need to get a

        date_columns: Optional[List[str]] = None,
        date_format: Optional[Union[str, Dict[str, str]]] = None,

set of parameters too?

A CSV is valid to be passed into this method.

# check if the column holds mixed timezones/offsets by extracting the UTC
# offsets; null values have no offset to extract and are dropped so they
# don't get counted as a spurious second "offset"
offsets = df[col].str.extract(r"([Zz\+\-]\d{2}:?\d{2})$")[0].dropna().unique()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
offsets = df[col].str.extract(r"([Zz\+\-]\d{2}:?\d{2})$")[0].dropna().unique()
offsets = df[col].astype("string").str.extract(r"([Zz\+\-]\d{2}:?\d{2})$")[0].dropna().unique()

There is a small edge case here. To recreate this:

import pandas as pd
import io
from synapseclient import Synapse
from synapseclient.models.mixins.table_components import _parse_df_date_cols_to_datetime

syn = Synapse()

# mimic csv_to_pandas_df's read_csv(...).convert_dtypes()
df = pd.read_csv(io.StringIO("col1,date_col\na,\nb,\n")).convert_dtypes()
print(df["date_col"].dtype)  # Int64

res = _parse_df_date_cols_to_datetime(df, date_columns=["date_col"], synapse_client=syn)
print(res["date_col"].dtype)  # datetime64[ns] is what is expected here after patching
# -> AttributeError: Can only use .str accessor with string values!
    raise AttributeError("Can only use .str accessor with string values!")
AttributeError: Can only use .str accessor with string values!. Did you mean: 'std'?

@BryanFauble BryanFauble left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few small considerations that are required - However, this is looking great.

# ROW_ID/ROW_VERSION must stay as regular columns so they survive the
# round-trip to the temporary upload file when `date_columns` is used —
# dropping them would turn a row update into an append.
values = csv_to_pandas_df(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this use the same parameters as df.to_csv at line 4971? For example, this test currently fails:

async def test_store_rows_async_csv_date_cols_respects_non_default_separator(self):
        # GIVEN a TAB-separated CSV and a matching csv_table_descriptor telling
        # the client the file is tab-delimited
        table = self.ClassForTest(id="syn123", name="test_table")
        table._last_persistent_instance = self.ClassForTest(
            id="syn123", name="test_table"
        )
        with tempfile.NamedTemporaryFile(
            mode="w", suffix=".csv", delete=False
        ) as csv_file:
            csv_file.write("col1\tdate_col\na\t01/15/2024\nb\t02/20/2024\n")

        uploaded = {}

        def capture_upload(**kwargs):
            # The temp upload file is written with the descriptor's separator,
            # so read it back with the same separator
            uploaded["df"] = pd.read_csv(kwargs["path_to_csv"], sep="\t")

        try:
            with patch.object(
                table, "_chunk_and_upload_csv", new_callable=AsyncMock
            ) as mock_upload:
                mock_upload.side_effect = capture_upload
                # WHEN the tab-delimited file is stored with date_columns
                await table.store_rows_async(
                    values=csv_file.name,
                    date_columns=["date_col"],
                    date_format="%m/%d/%Y",
                    csv_table_descriptor=CsvTableDescriptor(separator="\t"),
                    synapse_client=self.syn,
                )

            # THEN the file is parsed with the tab separator, so date_col is
            # found and converted to epoch ms — rather than the whole header
            # collapsing into a single "col1\tdate_col" column (which raises a
            # "date column(s) not present" ValueError)
            expected_df = pd.DataFrame(
                {
                    "col1": ["a", "b"],
                    "date_col": [
                        1705276800000,
                        1708387200000,
                    ],  # date columns are converted to epoch ms (midnight local timezone, unit tests run with TZ=UTC)
                }
            ).convert_dtypes()
            pd.testing.assert_frame_equal(
                uploaded["df"], expected_df, check_dtype=False
            )
        finally:
            os.remove(csv_file.name)

@danlu1 danlu1 Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root cause is that the input data isn't being parsed correctly. It only has one column col1\tdate_col'

df.to_csv(
temp_path,
index=False,
float_format="%.12g",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the reason for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the convention of writing csv in the client. Here is the reference:

# NOTE: reason for flat_format='%.12g':
# pandas automatically converts int columns into float64 columns when some cells in the column have no
# value. If we write the whole number back as a decimal (e.g. '3.0'), Synapse complains that we are writing
# a float into a INTEGER(synapse table type) column. Using the 'g' will strip off '.0' from whole number
# values. pandas by default (with no float_format parameter) seems to keep 12 values after decimal, so we
# use '%.12g'.c
# see SYNPY-267.

3 S4 2017-02-14 07:00:00+00:00
4 S5 2017-02-14 19:23:00+00:00
5 S6 2018-10-01 16:30:00+00:00
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these correct Los Angeles times?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these are printed results of the above code.

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.

4 participants