[SYNPY-1632] Deprecate tables from the Synapse class and table.py module #1233

linglp · 2025-08-14T21:20:13Z

Problem:

Deprecate the following:

tableQuery
_queryTable
_queryTableNext
_uploadCsv
_check_table_transaction_response
_queryTableCsv
downloadTableColumns
_build_table_download_file_handle_list
_get_default_view_columns
_get_annotation_view_columns

Solution:

Map the following to new methods:

tableQuery -> query or query_async
_queryTable -> query_part_mask
_queryTableNext -> query_part_mask
_uploadCsv -> _chunk_and_upload_csv
_check_table_transaction_response -> internal function, no replacement
_uploadCSV -> store_rows_async
_queryTableCSV -> query_async (with downloadLocation parameter)
downloadTableColumns -> not seeing a direct replacement?
_build_table_download_file_handle_list -> internal function, no replacement
_get_default_view_columns -> internal function, no replacement
_get_annotation_view_columns -> internal function, no replacement

New data classes:

SumFileSizes
QueryResultOutput (created by using the old QueryBundleRequest)
Row
RowSet
SelectColumn
ActionRequiredCount
Query
QueryResult
QueryResultBundle
QueryNextPageToken
QueryJob
QueryBundleRequest

Testing:

Make sure all the new data classes have unit tests
Added TestQueryTableRowSet to test _query_table_row_set
Added TestQueryTableNextPage to test _query_table_next_page
Added TestQueryTableCsv to test _query_table_csv

linglp · 2025-08-15T22:34:25Z

synapseclient/models/mixins/table_components.py

+                list_columns = []
+                dtype = {}
+
+                for select_column in self.headers:


How to deal with self.headers?

headers for this logic is coming from the response of this API:
https://rest-docs.synapse.org/rest/org/sagebionetworks/repo/model/table/DownloadFromTableResult.html

We shouldn't need to store this data onto the Table-like class, so once you get the DownloadFromTableResult, you should be able to pass the result everywhere it's needed like this asDataFrame method. It shouldn't need to be exposed to the user querying for data on the Synapse Tables.

I reviewed the current code and it looks like headers go through two transformations:

In this section, the headers from DownloadFromTableResult (initially dictionaries) are converted into SelectColumn objects.

Later, in this section and this one, the column headers are transformed again.

If we’re not storing this information in CsvResult (or CsvFileTable), do we still need to convert headers into SelectColumn objects? Or are we moving away from SelectColumn entirely and planning to just treat headers as dictionaries?

To match our current behavior, I have:

class CsvResult: def __init__(self, file_path, include_row_id_and_row_version=True): self.file_path = file_path self.include_row_id_and_row_version = include_row_id_and_row_version if result and result.get("headers"): headers = result.get("headers") headers = [SelectColumn(**header) for header in headers] self.headers = self.set_column_headers(headers) else: self.headers = None

Based on your suggestion, it sounds like we don’t need self.headers at all. Instead, in asDataFrame we could just do something like:

if result.get("headers") is not None: headers = result["headers"] for column in headers: xxx

The only thing that we need to return back to the user in the case that they call
query_async -> The DataFrame, OR the path to the downloaded CSV
query_part_mask_async -> The results wrapped in the QueryResultBundle object

We do not have to maintain how the previous code worked at all for any of the intermediate objects or class types.

With that being said, in our case:

If we’re not storing this information in CsvResult (or CsvFileTable), do we still need to convert headers into SelectColumn objects? Or are we moving away from SelectColumn entirely and planning to just treat headers as dictionaries?

We do not need to maintain a CsvResult, SelectColumn, or any of the concepts the original table had implemented. In fact - The new Tables class also got rid of the SchemaBase and inheritance structure that was previous in place.

Based on your suggestion, it sounds like we don’t need self.headers at all. Instead, in asDataFrame we could just do something like:

if result.get("headers") is not None: headers = result["headers"] for column in headers: xxx

That is exactly right, we could do something like that.

In the end - If we are not exposing the interface to an end user, we have quite a bit more flexibility with how we need to maintain the "guts", or actual implementation of the function/method. However - Consider this, if it makes our lives easier for development, then there is no harm in creating dataclasses/classes for ourselves.

…g rowset

…ter match the original function

…ble_next_page to return QueryResultBundle; add test for _query_table_next_page

…ake sure the output type align with the async counterpart

… in QueryJob to align with synapse doc;

…for QUERY_RESULT

…with to_synapse_request

linglp · 2025-09-02T21:54:39Z

synapseclient/models/table_components.py

            "quoteCharacter": self.quote_character,
            "escapeCharacter": self.escape_character,
            "lineEnd": self.line_end,
-            "isFirstLineHeader": self.is_file_line_header,


This is a typo in the original CsvTableDescriptor class. It should be is_first_line_header

linglp · 2025-09-02T21:55:33Z

synapseclient/models/table_components.py

+    headers: Optional[List[SelectColumn]] = None
+    """The list of SelectColumns that describes the rows of this set."""
+
+    rows: Optional[List[Row]] = field(default_factory=list)


Initialize this as a list per conversation

linglp · 2025-09-03T15:28:41Z

synapseclient/models/mixins/table_components.py

+    query_job_request = QueryJob(
+        entity_id=entity_id,
+        sql=query,
+        write_header=header,


The definition of write_header based on the documentaiton of DownloadFromTableRequest is: Should the first line contain the columns names as a header in the resulting file? Set to 'true' to include the headers else, 'false'. The default value is 'true'.

There's also a isFirstLineHeader parameter under CsvTableDescriptor. I think both parameters mean the same thing. As you can see here, I have both: is_first_line_header=header, and is_first_line_header=header

thomasyu888

🔥 LGTM! Im going to defer to @BryanFauble for final review, but thanks for doing the giant deprecation.

Will there be a tutorial page we are going to update to use the new functions?

Copilot

Pull Request Overview

This PR deprecates multiple methods from the Synapse class and table.py module while introducing new table component data classes as part of modernizing the table querying API. The changes map deprecated methods to new implementations and provide comprehensive test coverage for the new functionality.

Deprecated 11 methods from synapseclient.client.Synapse class related to table operations
Added 11 new data classes for structured table operations: SumFileSizes, QueryResultOutput, Row, RowSet, SelectColumn, ActionRequiredCount, Query, QueryResult, QueryResultBundle, QueryNextPageToken, QueryJob, QueryBundleRequest
Migrated table query functionality to use new async-based implementations

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/synapseclient/mixins/unit_test_table_components.py	Added comprehensive unit tests for all new data classes and query functions
tests/integration/synapseclient/models/synchronous/test_table.py	Updated test expectations to reflect changes in batch processing behavior
tests/integration/synapseclient/models/async/test_table_async.py	Updated test expectations and refined spy behavior for async table operations
tests/integration/synapseclient/models/async/test_entityview_async.py	Enhanced test to capture call stack information for better verification
synapseclient/table.py	Added deprecation warnings to row_labels functions
synapseclient/models/table_components.py	Added 11 new data classes with complete REST API mappings and type conversions
synapseclient/models/mixins/table_components.py	Implemented new query functions and converted existing methods to use new data structures
synapseclient/models/mixins/asynchronous_job.py	Added endpoint mappings for new query request types
synapseclient/models/init.py	Exported new data classes for public API
synapseclient/core/constants/concrete_types.py	Added concrete type constants for new REST API models
synapseclient/client.py	Added deprecation decorators and migration examples to 11 deprecated methods
docs/reference/experimental/sync/table.md	Added documentation references for new data classes
docs/reference/experimental/async/table.md	Added documentation references for new data classes
.pre-commit-config.yaml	Updated bandit version from 1.7.5 to 1.8.0

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

synapseclient/models/table_components.py

tests/unit/synapseclient/mixins/unit_test_table_components.py

Copilot · 2025-09-03T15:42:22Z

synapseclient/models/table_components.py

+    This result is modeled from: <https://rest-docs.synapse.org/rest/org/sagebionetworks/repo/model/table/QueryResultBundle.html>
+    """
+
+    concrete_type: str = QUERY_TABLE_CSV_REQUEST


The default concrete_type for QueryResultBundle should be the bundle type, not the CSV request type. This should be QUERY_BUNDLE_REQUEST or a dedicated query result bundle constant, not QUERY_TABLE_CSV_REQUEST.

Suggested change

concrete_type: str = QUERY_TABLE_CSV_REQUEST

concrete_type: str = QUERY_BUNDLE_REQUEST

…date docstring

BryanFauble · 2025-09-03T16:59:39Z

🔥 LGTM! Im going to defer to @BryanFauble for final review, but thanks for doing the giant deprecation.

Will there be a tutorial page we are going to update to use the new functions?

Yes, https://sagebionetworks.jira.com/browse/SYNPY-1377 should capture the work to write up the tutorial page.

@linglp Do you mind reviewing this jira and add onto the topic for anything that we should cover in addition to whats there (if anything),

BryanFauble

I appreciate all your hard work that you've put into this!

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…psePythonClient into synpy-1632

linglp · 2025-09-05T14:11:32Z

I appreciate all your hard work that you've put into this!

I appreciate your review. Thank you again, Bryan!

Lingling Peng added 10 commits August 13, 2025 12:47

deprecate tableQuery

62d1968

add docstring to _queryTable and _queryTableNext for deprecation

89805fb

add deprecation message for _queryTableCsv

b42d058

add deprecation message for _uploadCSV

e506408

use standardized format

5c04e36

add deprecation notice to private functions

9f5d1f0

Merge branch 'develop' into synpy-1632

33d6e7d

Merge branch 'develop' into synpy-1632

cb6cd78

migrate selectColumn, queryTablecsv, and tableQuery

73eee3f

change to snake case

546181b

linglp commented Aug 15, 2025

View reviewed changes

Lingling Peng added 19 commits August 15, 2025 18:38

handle headers

3bbf6e9

add default; also fix headers

7df37b2

change function name to snake case

3d3c4cd

fix

a793730

make synapse client optional in table_query

c072ada

merge with develop

ad88b7b

use csv_to_pandas_df directly, avoid using TableQueryResult when usin…

4b964cb

…g rowset

use csv_to_pandas_df directly, avoid using TableQueryResult when usin…

59b9ae3

…g rowset

bring back QueryResultBundle, add docstring, turn functions to internal

847a499

add data classes and fix tests

42df8dd

turn query_table_csv to async so that download_by_file_handle can bet…

e93c90e

…ter match the original function

add test for _query_table_csv

5287f88

make sure the default is None for all the dataclasses; make _query_ta…

632de50

…ble_next_page to return QueryResultBundle; add test for _query_table_next_page

fix test

d7bcf20

turn _query_table_csv back to a sync function and update test

846f537

clean up docstring and comment

0d94ccf

fix code since we are now using queryresultbundle

8f89c6e

correct type hinting. it should return a list of rows not rowset

437ae76

revert the function signature back for query_part_mask function and m…

4d441ff

…ake sure the output type align with the async counterpart

Lingling Peng added 9 commits August 29, 2025 16:52

add doc for sync

ff441b4

fix docstring; fix typos in CsvTableDescriptor; added more attributes…

259c1c7

… in QueryJob to align with synapse doc;

add docstring

58e67d1

change the rowset function to private

f5f7dd2

remove duplicated import

47e06fb

add unit test for _query_table_row_set function; add a concrete type …

80b7aaa

…for QUERY_RESULT

initialize rows in rowset as an empty list

02509c5

correct typo in attribute name; make sure to call csvtabledescriptor …

bbdd0a1

…with to_synapse_request

move import statement and also fix unit test

9a6fe4f

linglp commented Sep 2, 2025

View reviewed changes

edit docstring; add concrete type constant

b9fd1a5

linglp marked this pull request as ready for review September 2, 2025 22:04

linglp requested a review from a team as a code owner September 2, 2025 22:04

update the sync test

17782f0

linglp commented Sep 3, 2025

View reviewed changes

thomasyu888 reviewed Sep 3, 2025

View reviewed changes

thomasyu888 requested a review from Copilot September 3, 2025 15:38

Copilot AI reviewed Sep 3, 2025

View reviewed changes

Lingling Peng added 2 commits September 3, 2025 11:42

fix test in sync folder just like async; remove confusing comment; up…

130e9fc

…date docstring

fix test in sync

57010cf

BryanFauble approved these changes Sep 4, 2025

View reviewed changes

linglp and others added 4 commits September 4, 2025 15:51

fix typo

f3a3cb6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

remove unused commit

27cd5ab

Merge branch 'synpy-1632' of https://github.com/Sage-Bionetworks/syna…

bd55571

…psePythonClient into synpy-1632

add back greater_than due to error from copilot

daa99bf

linglp merged commit 0b11782 into develop Sep 5, 2025
28 checks passed

linglp deleted the synpy-1632 branch September 5, 2025 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYNPY-1632] Deprecate tables from the Synapse class and table.py module #1233

[SYNPY-1632] Deprecate tables from the Synapse class and table.py module #1233

Uh oh!

linglp commented Aug 14, 2025 •

edited

Loading

Uh oh!

linglp Aug 15, 2025 •

edited

Loading

Uh oh!

BryanFauble Aug 18, 2025

Uh oh!

linglp Aug 18, 2025

Uh oh!

BryanFauble Aug 18, 2025

Uh oh!

linglp Sep 2, 2025

Uh oh!

linglp Sep 2, 2025

Uh oh!

linglp Sep 3, 2025

Uh oh!

thomasyu888 left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

BryanFauble commented Sep 3, 2025

Uh oh!

BryanFauble left a comment

Uh oh!

linglp commented Sep 5, 2025

Uh oh!

Uh oh!

Uh oh!

	concrete_type: str = QUERY_TABLE_CSV_REQUEST
	concrete_type: str = QUERY_BUNDLE_REQUEST

[SYNPY-1632] Deprecate tables from the Synapse class and table.py module #1233

[SYNPY-1632] Deprecate tables from the Synapse class and table.py module #1233

Uh oh!

Conversation

linglp commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem:

Solution:

New data classes:

Testing:

Uh oh!

linglp Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BryanFauble Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

linglp Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

BryanFauble Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

linglp Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

linglp Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

linglp Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

thomasyu888 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

BryanFauble commented Sep 3, 2025

Uh oh!

BryanFauble left a comment

Choose a reason for hiding this comment

Uh oh!

linglp commented Sep 5, 2025

Uh oh!

Uh oh!

Uh oh!

linglp commented Aug 14, 2025 •

edited

Loading

linglp Aug 15, 2025 •

edited

Loading