Skip to content

Conversation

@smaheshwar-pltr
Copy link
Contributor

Rationale for this change

Silence PyCharm warning that DataScan::_check_sequence_number can be static.

Are these changes tested?

N/A

Are there any user-facing changes?

No

Comment on lines +1860 to +1861
@staticmethod
def _check_sequence_number(min_sequence_number: int, manifest: ManifestFile) -> bool:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Silencing PyCharm warning that DataScan::_check_sequence_number can be static. Could have alternatively lifted the method out of the class. Not strongly opinionated between the two

@smaheshwar-pltr smaheshwar-pltr marked this pull request as ready for review July 10, 2025 12:52
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM

this is also only used once in the same file, so i think its fine to leave it as is

@kevinjqliu kevinjqliu merged commit 6c6df96 into apache:main Jul 10, 2025
10 checks passed
gabeiglio pushed a commit to Netflix/iceberg-python that referenced this pull request Aug 13, 2025
<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change

Silence PyCharm warning that `DataScan::_check_sequence_number` can be
static.

# Are these changes tested?

N/A

# Are there any user-facing changes?

No

<!-- In the case of user-facing changes, please add the changelog label.
-->

Co-authored-by: Sreesh Maheshwar <smaheshwar@palantir.com>
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.

2 participants