Repository navigation
[SYNPY-1933] Updated pandas to use 3.x - #1471
andrewelamb wants to merge 6 commits into
Conversation
andrewelamb
left a comment
There was a problem hiding this comment.
Comments that explain the code changes in synapseclient/ and synapseutils/. Refer to the problem numbers in the PR description.
| ).fillna(False) | ||
| # Series.map is used per column because DataFrame.map needs pandas 2.1 or | ||
| # later, and DataFrame.applymap was removed in pandas 3 | ||
| ints = dataframe.apply( |
There was a problem hiding this comment.
Problems 4 and 8. DataFrame.applymap is removed in pandas 3, and DataFrame.map is not in pandas < 2.1. Series.map is in all supported versions, thus we apply it to each column. This code path is used for all manifest sizes.
The large-manifest path used pandarallel.parallel_applymap, which does not work with pandas 3. Thus pandarallel is removed from this function, from check_curator_imports(), and from the dependencies.
|
|
||
| # Identify cells converted to integers | ||
| is_int = ints.map(is_integer) # type: ignore | ||
| is_int = ints.apply(lambda col: col.map(is_integer)) |
There was a problem hiding this comment.
Same change as above: ints.map(is_integer) (DataFrame.map, pandas >= 2.1) changes to Series.map on each column, so that it works on pandas 1.5 to 3.x.
|
|
||
| # replace values that couldn't be converted to float with the original str values | ||
| float_df[col].fillna(dataframe[col][float_df[col].isna()], inplace=True) | ||
| float_df[col] = float_df[col].fillna(dataframe[col][float_df[col].isna()]) |
There was a problem hiding this comment.
Problem 3. In pandas 3, Copy-on-Write is always on. float_df[col].fillna(..., inplace=True) changes a temporary copy and not float_df, so the original string values were not put back. We now assign the result back to the column.
| import pandas as pd | ||
|
|
||
| def construct_rownames(query_result_bundle, offset=0): | ||
| def construct_rownames(rowset, offset=0): |
There was a problem hiding this comment.
Problem 7. construct_rownames now accepts a RowSet, not a QueryResultBundle. Before this change, the function read query_result_bundle.query_result.query_results.rows for every page. Later pages are RowSet objects, so a result with more than one page raised AttributeError when the row ID and version were in the index (the default). The KeyError fallback also used rowset["rows"], but a RowSet is a dataclass, so we use rowset.rows.
| # TODO: Look into why this isn't being assigned | ||
| series["ROW_ID"].append( | ||
| pd.Series(name="ROW_ID", data=[row.id for row in rowset.rows]) | ||
| series["ROW_ID"] = pd.concat( |
There was a problem hiding this comment.
Problem 7. Series.append is removed in pandas 2.0. Also, the old code did not assign the result (see the removed TODO), so the rows from later pages were not added to ROW_ID and ROW_VERSION. We now use pd.concat and assign the result.
The key also changes from row.id to row.row_id, which is the correct field name on the Row model.
| # pandas 3 reports "str" for text columns; keep "object" | ||
| stats[column]["dtype"] = ( | ||
| "object" | ||
| if pd.api.types.is_string_dtype(df[column].dtype) |
There was a problem hiding this comment.
Problem 6. In pandas 3, dtype.name for a text column is "str", not "object". We report "object" when is_string_dtype() is true, so that the output of describe() does not change for users.
| def _sortAndFixProvenance(syn, df): | ||
| df = df.set_index("path") | ||
| # Lists are written into these cells, which a str dtype column cannot hold | ||
| for col in PROVENANCE_FIELDS: |
There was a problem hiding this comment.
Problem 1 (legacy path). Same fix as in models/services/manifest.py: the used and executed columns change to object dtype, so that lists can be written into the cells on pandas 3.
| if "synapseStore" not in df: | ||
| df = df.assign(synapseStore=None) | ||
| # A blank column is read as float64, which cannot hold bool values | ||
| df["synapseStore"] = df["synapseStore"].astype(object) |
There was a problem hiding this comment.
Problem 2 (legacy path). Same fix as in _apply_synapse_store_defaults: a blank synapseStore column is read as float64, so we change it to object dtype before True/False is set.
| file, | ||
| row["used"] if "used" in row else [], | ||
| row["executed"] if "executed" in row else [], | ||
| _missing_to_empty_list(row["used"]) if "used" in row else [], |
There was a problem hiding this comment.
Problem 1. On pandas 3, iterrows() gives a str-dtype row when all values in the row are strings or missing, and a missing used/executed value becomes NaN, not None. The upload iterates over these values, so NaN caused an error. _missing_to_empty_list() changes None or NaN to [].
| return True | ||
|
|
||
|
|
||
| def _missing_to_empty_list(value): |
There was a problem hiding this comment.
New helper for the change above. It checks float + math.isnan instead of pd.isna, because pd.isna on a list returns an array and not a bool.
andrewelamb
left a comment
There was a problem hiding this comment.
Comments that explain the tests that this PR adds or changes. Refer to the problem numbers in the PR description.
| assert expected_indicies == dataframe.index.values.tolist() | ||
|
|
||
|
|
||
| def _multi_page_query_results() -> tuple[dict, list[dict]]: |
There was a problem hiding this comment.
New helper (problem 7, legacy API). This helper makes a mocked table query with 3 pages: a first QueryResultBundle with 2 rows, then 2 next QueryResult pages with 2 rows and 1 row. Each row has a different rowId, etag and value, so the tests can make sure that the rows of all pages are in the DataFrame, in the correct order. The rows use the REST keys rowId and versionNumber, which the fix in table.py now reads.
| return query_result, next_pages | ||
|
|
||
|
|
||
| def test_rowset_asDataFrame__multiple_pages_row_id_and_version_not_in_index( |
There was a problem hiding this comment.
New test (problem 7, legacy API). With rowIdAndVersionInIndex=False, this test makes sure that ROW_ID, ROW_VERSION and ROW_ETAG have the values of all 3 pages. Before the fix, Series.append did not assign its result (and it is removed in pandas 2), and ROW_ETAG had the etags of the last page only.
| assert [f"value{i}" for i in range(5)] == list(dataframe["annotationColumn1"]) | ||
|
|
||
|
|
||
| def test_rowset_asDataFrame__multiple_pages_row_id_and_version_not_in_index_has_unique_index( |
There was a problem hiding this comment.
New test (problem 7, legacy API). This test makes sure that the index is unique and is 0 to 4 across all pages. Before the fix, each page numbered its rows from 0, so the index had duplicate values.
| assert result.select_columns[0].id == "12345" | ||
|
|
||
|
|
||
| class TestRowsetToPandasDf: |
There was a problem hiding this comment.
New test class (problem 7, new API). These tests cover _rowset_to_pandas_df with more than one page. Before this PR, no unit test covered this path. _make_bundle makes a QueryResultBundle from the dataclass models (Row, RowSet, QueryResult). _query_table_next_page is mocked to return the next pages, so no network call is made.
| ) | ||
| ) | ||
|
|
||
| def test_multiple_pages_row_id_and_version_not_in_index(self) -> None: |
There was a problem hiding this comment.
New test. With row_id_and_version_in_index=False, this test makes sure that all pages are fetched, and that ROW_ID, ROW_VERSION, ROW_ETAG and the data columns have the values of all pages. This covers the pd.concat change and the row.row_id key fix.
| include_row_id_and_row_version=False, | ||
| ) | ||
|
|
||
| # dtype=object keeps None for missing values, matching the query output, on all |
There was a problem hiding this comment.
Changed (test fix only). In test_store_rows_from_csv_with_date_columns, the query gives an object column with None for the missing date. Without dtype=object, the expected column was float64 with NaN, and on pandas 3 the comparison failed with None != nan. With dtype=object, the missing value stays None on all pandas versions. The client code does not change.
| include_row_id_and_row_version=False, | ||
| ) | ||
|
|
||
| # dtype=object keeps None for missing values, matching the query output, on all |
There was a problem hiding this comment.
Changed (test fix only). In test_store_rows_from_csv_with_tz_aware_date_columns, the query gives an object column with None for the missing date. Without dtype=object, the expected column was float64 with NaN, and on pandas 3 the comparison failed with None != nan. With dtype=object, the missing value stays None on all pandas versions. The client code does not change.
| check_names=True, | ||
| check_dtype=False, | ||
| ) | ||
| # dtype=object keeps None for missing values, matching the query output, |
There was a problem hiding this comment.
Changed (test fix only). In test_update_rows_and_annotations, the query gives None for missing values in integer_column and float_column. The expected series used the default dtype, so a missing value became NaN, and on pandas 3 the comparison failed with None != nan. Both series now use dtype=object. The client code does not change.
| if os.name == "nt": | ||
| df["datetime64"] = pd.to_datetime(df["datetime64"], utc=True) | ||
| df["datetime64"] = pd.to_datetime(df["datetime64"], utc=True).astype( | ||
| "datetime64[ns, UTC]" |
There was a problem hiding this comment.
Changed (test fix only). The client returns DATE columns as datetime64[ns, UTC] (see the change in _convert_df_date_cols_to_datetime). On pandas 3, pd.to_datetime can give a different resolution, for example datetime64[ms, UTC], so the dtype check failed. The expected column is now cast to datetime64[ns, UTC]. This line is the Windows path.
| df["datetime64"] = df["datetime64"].astype(int) | ||
| df["datetime64"] = pd.to_datetime(df["datetime64"], unit="ms", utc=True) | ||
| df["datetime64"] = pd.to_datetime(df["datetime64"], unit="ms", utc=True).astype( | ||
| "datetime64[ns, UTC]" |
There was a problem hiding this comment.
Same change as above, for the non-Windows path (unit="ms").
95d3a79 to
dce02d2
Compare
Problem:
setup.cfgpinspandas>=1.5,<3.0. No pandas 2.x release has a Python 3.15 wheel. pandas 3.0.6 is the first release that supports Python 3.11 through 3.15.pd.to_datetime(synapseclient/table.py). Thus the client must support pandas 3 before it can support Python 3.15.Solution:
The pandas requirement changes to
pandas>=1.5,<4.0synapseclient/models/services/manifest.py,synapseutils/sync.pyusedandexecutedcolumns toobjectdtype before lists are written into the cells. New helper_missing_to_empty_list()insync.pychanges aNoneorNaNvalue inusedorexecutedto an empty list, so that the upload can iterate over it.manifest.py(_apply_synapse_store_defaults),sync.py(readManifestFile_async)synapseStorecolumn toobjectdtype beforeTrue/Falseis set.synapseclient/extensions/curator/schema_generation.py(convert_floats)float_df[col] = float_df[col].fillna(...).schema_generation.py(find_and_convert_ints)dataframe.apply(lambda col: col.map(...))for all manifests. pandarallel is removed (see below).synapseclient/table.py(_convert_df_date_cols_to_datetime).astype("datetime64[ns, UTC]").synapseutils/describe_functions.py(_describe_wrapper)"object"ifis_string_dtype()is true, elsedtype.name.table.py,synapseclient/models/mixins/table_components.pypd.concatreplacesSeries.append. Use the correct keys on all pages. AppendROW_ETAG. When the row ID and version are not in the index, the final DataFrame is renumbered withreset_index(drop=True), so that the index is unique across all pages. Intable_components.py,construct_rownamesnow accepts aRowSet. Before this change, a result with more than one page raisedAttributeErrorwhen the row ID and version were in the index (the default).Other changes
pandarallel is removed
Pipfile.lockis regenerated (pipenv lock). pandas 2.3.3 changes to 3.0.6.pytz,tzdata,pandarallelanddillare removed from thedevelopsection. Other packages have small version steps.New CI job
test-pandas-versionsin.github/workflows/build.yml. It runs the unit tests onubuntu-22.04with older pandas versions. Thepackagejob now needs this job.<2setup.cfg.<2DataFrame.mapand Python 3.12 wheels.pandas < 2.2.2 is not compatible with numpy 2, thus each entry sets its own numpy requirement. This is a separate job, so that the integration tests do not run on pandas 1.5.
CLAUDE.mdnow has the pandas compatibility rules and a description of the new CI job.Testing:
New unit tests:
TestRowsetToPandasDf::test_multiple_pages_row_id_and_version_not_in_indexTestRowsetToPandasDf::test_multiple_pages_row_id_and_version_not_in_index_has_unique_indexTestRowsetToPandasDf::test_multiple_pages_row_id_and_version_in_indextest_rowset_asDataFrame__multiple_pages_row_id_and_version_not_in_indextest_rowset_asDataFrame__multiple_pages_row_id_and_version_not_in_index_has_unique_indexsync.py)test_manifest_upload_missing_provenance_builds_dependency_graphmanifest.py)TestApplySynapseStoreDefaults::test_blank_column_read_from_csvsync.py)test_read_manifest_file_synapse_store_values_all_blankChanged unit tests:
TestCsvToPandasDf(4 tests):.astype("datetime64[ns, UTC]")is added to the expected date values. The client givesdatetime64[ns, UTC], butpd.to_datetime(..., unit="ms")givesdatetime64[ms, UTC]on pandas 3, so the old expected values conflicted with the dtype check.TestParseDfDateColsToDatetime::test_uniform_utc_offsets_parsed_as_tz_aware: the test now checks the UTC offset, not the timezone class. pandas 1.5 givespytz.FixedOffset, and newer versions givedatetime.timezone.Changed integration tests:
With pandas 3.0.6, 5 integration tests failed. These are test fixes only. The client code does not change.
test_table_async.py::TestRowStorage::test_store_rows_from_df_with_datetime_columns,test_store_rows_from_csv_with_date_columns,test_store_rows_from_csv_with_tz_aware_date_columnsNone != nan. The query gives anobjectcolumn withNonefor the missing date. The expected column wasfloat64withNaN.dtype=object, so the missing value staysNoneon all pandas versions.test_entityview_async.py::TestEntityView::test_update_rows_and_annotationsNone != nanininteger_column.integer_columnandfloat_columnseries usedtype=object.deprecated/test_tables.py::test_tables_pandasdtypeis different:datetime64[ns, UTC](client) vsdatetime64[ms, UTC](expected).datetime64column is changed todatetime64[ns, UTC]on the Windows and non-Windows paths.