Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds stub generation support by providing .pyi files, enforcing direct instantiation guards in C++ bindings, and aligning the return value of cluster.find with cluster.query.
- Introduces a new stub module (
.pyi) for the QuasarDB Python API. - Replaces default constructors on protected classes with lambdas that throw
DirectInstantiationError. - Updates
cluster.findto return astd::vector<std::string>directly and adjusts the binding.
Reviewed Changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| quasardb/quasardb/__init__.pyi | New stub file defining the public Python API |
| properties.hpp, perf.hpp, options.hpp, … | Modified bindings to throw DirectInstantiationError in py::init lambdas |
| cluster.hpp / cluster.cpp | Changed find to return a vector<string> and updated its binding |
| module.cpp | Initialized never_expires via Python’s datetime.fromtimestamp |
| dev-requirements.txt | Added mypy and pybind11-stubgen in development requirements |
Comments suppressed due to low confidence (3)
quasardb/cluster.hpp:340
- [nitpick] The variable name
ois vague; consider renaming it to something more descriptive likequery_objorfinder.
auto o = std::make_shared<qdb::find_query>(_handle, query_string);
quasardb/cluster.hpp:335
- Add or update unit tests for the new
findimplementation that directly returnsstd::vector<std::string>to verify correct behavior.
qdb::find_query find(const std::string & query_string)
quasardb/quasardb/init.pyi:83
- The
metricssymbol is included in__all__but never imported; either add its import (e.g.from ._perf import metrics) or remove it from__all__.
"metrics",
solatis
approved these changes
Jul 3, 2025
rodp63
added a commit
that referenced
this pull request
Jul 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Worked with
pybind11-stubgenas base tool, which produced more accurate results thanstubgenfrommypy.Summary of Changes
.pyistub files for all exposed C++ modules.qdb::handlein their constructors can no longer be directly instantiated. This is enforced via the following binding:.def(py::init([](py::args, py::kwargs) { throw qdb::direct_instantiation_exception{}; return nullptr; }))quasardbstub module to prevent direct instantiation. They still can be imported at runtime, but no type hints will be shown.conn.find()now returns the result directly, aligning its behavior withconn.query()for consistency.