Skip to content

feat!: refactor Dataset#config api and expose it via pylance #4041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 25, 2025

Conversation

yanghua
Copy link
Collaborator

@yanghua yanghua commented Jun 19, 2025

BREAKING CHANGE: the config API's signature from:

pub fn config(&self) -> impl Iterator<Item = (&String, &String)>

To

pub fn config(&self) -> Result<HashMap<String, String>>

@github-actions github-actions bot added the enhancement New feature or request label Jun 19, 2025
@yanghua yanghua force-pushed the feat-expose-get-config branch from 6741aff to 15c00f8 Compare June 19, 2025 12:13
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.18%. Comparing base (159f0ec) to head (12f576c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4041   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files         300      300           
  Lines      114101   114101           
  Branches   114101   114101           
=======================================
+ Hits        90347    90350    +3     
+ Misses      20272    20270    -2     
+ Partials     3482     3481    -1     
Flag Coverage Δ
unittests 79.18% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yanghua yanghua requested a review from jackye1995 June 19, 2025 15:41
@yanghua yanghua marked this pull request as ready for review June 19, 2025 23:39
@yanghua
Copy link
Collaborator Author

yanghua commented Jun 19, 2025

cc @jackye1995 , marked the PR as ready for review

@jackye1995
Copy link
Contributor

Any update on this?

@yanghua yanghua force-pushed the feat-expose-get-config branch from 6bc560f to 268b98c Compare June 25, 2025 05:16
@yanghua yanghua changed the title feat: expose get_config api feat!: refactor Dataset#config api Jun 25, 2025
@yanghua yanghua force-pushed the feat-expose-get-config branch from 268b98c to 5236468 Compare June 25, 2025 05:23
Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

Looks good to me, pending CI passing. Also the current description seems empty, we should describe the breaking change.

@yanghua yanghua changed the title feat!: refactor Dataset#config api feat!: refactor Dataset#config api and expose it via pylance Jun 25, 2025
@yanghua
Copy link
Collaborator Author

yanghua commented Jun 25, 2025

Traceback (most recent call last):
New version: (0, 30, 1)
Last version: (0, 30, 0)
No breaking changes found.
  File "/home/runner/work/lance/lance/pr/../base/ci/check_versions.py", line 77, in <module>
    new_version[1] == last_version[1] + 1
AssertionError: Minor version should have been bumped because there was a breaking change.
Error: Process completed with exit code 1.

@jackye1995 The breaking change policy failed. Shall we update the version? Or just ignore?

@jackye1995
Copy link
Contributor

Yeah if you are the first author to do breaking change after the last release, then you need to do the version bump

@github-actions github-actions bot added the java label Jun 25, 2025
@yanghua yanghua merged commit 8995a72 into lancedb:main Jun 25, 2025
38 of 39 checks passed
@yanghua yanghua deleted the feat-expose-get-config branch June 25, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants