Skip to content

Conversation

@jahnvi480
Copy link
Contributor

@jahnvi480 jahnvi480 commented Jul 29, 2025

Work Item / Issue Reference

AB#38059


Summary

This pull request refactors type objects in mssql_python/type.py to inherit from their respective Python built-in types, simplifies constructors, and updates related tests to align with the new implementation. Additionally, it modifies timestamp and time handling functions for better compatibility and enhances the Binary function to handle multiple input types.

Refactoring of type objects:

  • mssql_python/type.py: Changed type classes (STRING, BINARY, NUMBER, DATETIME, ROWID) to inherit from Python built-in types (str, bytearray, float, datetime.datetime, int) and replaced __init__ methods with __new__ methods for direct instantiation.

Updates to utility functions:

  • mssql_python/type.py: Modified TimeFromTicks to use time.localtime instead of time.gmtime and updated TimestampFromTicks to remove the UTC timezone. Enhanced Binary to handle both str and bytes inputs, with fallback for other types by converting to string first.

Updates to tests:

  • tests/test_002_types.py: Updated tests for type objects (STRING, BINARY, NUMBER, DATETIME, ROWID) to validate against their respective Python built-in types instead of custom attributes.
  • tests/test_002_types.py: Adjusted test_time_from_ticks and test_timestamp_from_ticks to reflect changes in TimeFromTicks and TimestampFromTicks behavior. Improved test_binary_constructor to test compatibility with both bytes and bytearray.

Copilot AI review requested due to automatic review settings July 29, 2025 11:07
@github-actions github-actions bot added the pr-size: small Minimal code update label Jul 29, 2025
Copy link
Contributor

Copilot AI left a comment

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 refactors type objects in mssql_python/type.py to inherit from Python built-in types and makes related utility functions more compatible with pyodbc standards.

  • Refactored type classes (STRING, BINARY, NUMBER, DATETIME, ROWID) to inherit from built-in Python types instead of custom classes
  • Modified timestamp and time utility functions to use local time instead of UTC
  • Enhanced the Binary function to handle multiple input types (str, bytes, and fallback for others)

Reviewed Changes

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

File Description
mssql_python/type.py Refactored type classes to inherit from built-ins, updated time functions to use local time, enhanced Binary function
tests/test_002_types.py Updated tests to validate new type inheritance behavior and adjusted time/timestamp test expectations

@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Jul 29, 2025
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Jul 29, 2025
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Jul 30, 2025
bewithgaurav
bewithgaurav previously approved these changes Jul 31, 2025
Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Left one comment on how to handle unexpected type in Binary func,

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Aug 1, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 5, 2025
@bewithgaurav bewithgaurav merged commit 6077554 into main Aug 5, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants