Codecov Report

Merging #24363 (67865bc) into master (37fb90c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #24363   +/-   ##
=======================================
  Coverage   73.35%   73.35%           
=======================================
  Files         718      718           
  Lines       97033    97033           
=======================================
+ Hits        71177    71180    +3     
+ Misses      24525    24522    -3     
  Partials     1331     1331           

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

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @AnandInguva for label python.
R: @johnjcasey for label io.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

run python precommit

LGTM, just rerunning python

This makes more sense, obviously. Just have worried that if, ever, some user apps could assume the (00123-of-00124) naming pattern and we will break them mysteriously. It may deserve to be noted in "breaking change".

Agreed with Yi. Can you add a note to the changes file?

I also suspect same issue also exists elsewhere in the code base. e.g. here:

Sorry, I mean the finding is great, however due to same issue elsewhere, partial fix would then make the indexing inconsistent throughout the code base.

Sorry, I mean the finding is great, however due to same issue elsewhere, partial fix would then make the indexing inconsistent throughout the code base.

Ah, ok. I have checked that this is used in fileio and in the dataframes module. I will update this PR with more details about the potential impact. So far, in my tests, this does not break anything anywhere else in the Python SDK.

Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.

Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.

Yes, ideally naming should be consistent throughout SDKs. Currently it is consistent (though weird). Would appreciate if you are willing to find the appearance in both the Java and Python SDKs.

Reminder, please take a look at this pr: @AnandInguva @johnjcasey

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @tvalentyn for label python.
R: @chamikaramj for label io.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

stop reviewer notifications

Stopping reviewer notifications for this pull request: requested by reviewer

Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.

Yes, ideally naming should be consistent throughout SDKs. Currently it is consistent (though weird). Would appreciate if you are willing to find the appearance in both the Java and Python SDKs.

@Abacn

I'd like to keep working on and change the Go and Java SDK. What should I do with Git? Should I create a new branch from master? Seams that I can not create a branch based on this branch(iht:minor_python_doc)

thanks for the interests. I would expect many places to change and imo limited benefit compared to the risk.

thanks for the interests. I would expect many places to change and imo limited benefit compared to the risk.

I am going to close this PR now, and I will be sending a new one with consistent changes between the Python and Java SDKs. After the holidays, this branch is outdated, and I prefer just to start over than resolving the conflcits.