Skip to content

Conversation

@solatis
Copy link
Contributor

@solatis solatis commented Jun 11, 2025

  • Constructing dataframe while providing index at the same time is much faster
  • We're now using named indexes, so users can refer to e.g. df['$timestamp'] and it will work
  • Added deprecation notices for unused parameters

@solatis solatis requested a review from Copilot June 11, 2025 02:40
Copy link

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

Optimize the query function to construct DataFrames with an index in one step, introduce named indexes, and deprecate unused parameters.

  • Directly pass the index to pd.DataFrame for faster construction
  • Use a named index ("$index" or the provided column name)
  • Add deprecation warnings for the now-ignored blobs and numpy arguments
Comments suppressed due to low confidence (5)

quasardb/pandas/init.py:187

  • Docstring is missing a Returns section—please document that the function returns a pandas.DataFrame.
    Execute *query* and return the result as a pandas DataFrame.

quasardb/pandas/init.py:201

  • Combine parameter names in the docstring is confusing; list blobs and numpy each with their own description line.
    blobs, numpy

quasardb/pandas/init.py:205

  • Add unit tests to verify that passing non-default values for blobs and numpy triggers a DeprecationWarning.
# ------------------------------------------------------------------ deprecations

quasardb/pandas/init.py:206

  • Inconsistent indentation: this if should align with the function’s 4-space indent level (remove the extra space before if).
     if blobs is not False:

quasardb/pandas/init.py:213

  • Inconsistent indentation: this if should align with the function’s 4-space indent level (remove the extra space before if).
     if numpy is not True:

df = pd.DataFrame(m)
index_vals, m = qdbnp.query(cluster, query, index=index, dict=True)

index_name = "$index" if index is None else index
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting the magic string "$index" into a module-level constant to improve clarity and avoid duplication.

Suggested change
index_name = "$index" if index is None else index
index_name = DEFAULT_INDEX_NAME if index is None else index

Copilot uses AI. Check for mistakes.
@solatis solatis requested review from rodp63 and vikonix June 11, 2025 02:42
@solatis solatis merged commit 2d72591 into master Jun 11, 2025
2 checks passed
@solatis solatis deleted the sc-16841/optimize-indexed-dataframe-construction-use-named-index-in-python-api branch June 11, 2025 08:27
solatis added a commit that referenced this pull request Jun 11, 2025
* Constructing dataframe while providing index at the same time is much faster
* We're now using named indexes, so users can refer to e.g. df['$timestamp'] and it will work
* Added deprecation notices for unused parameters
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.

3 participants