Skip to content

Conversation

@soffer-anyscale
Copy link
Contributor

Description

This PR fixes the import statement in MCAPDatasource to follow Python best practices for explicit imports.

Change: Updated from import mcap + mcap.reader.make_reader(f) to from mcap.reader import make_reader for cleaner, more explicit importing.

Why this is needed:

  • Follows Python best practices for explicit imports (PEP 8)
  • Improves code readability by making it clear what's being used
  • Better compatibility with different mcap package versions
  • Reduces namespace pollution

Related issues

None - this is a code quality improvement.

Additional information

What changed

Before:

import mcap

reader = mcap.reader.make_reader(f)

After:

from mcap.reader import make_reader

reader = make_reader(f)

Impact

  • Minimal risk: This is a refactoring of the import statement only
  • No API changes: The MCAPDatasource public API remains unchanged
  • No behavioral changes: The functionality is identical
  • Tested: All ruff lint checks pass

File modified

  • python/ray/data/_internal/datasource/mcap_datasource.py (2 lines changed)

This change makes the code more maintainable and follows modern Python conventions for explicit imports.

Change from 'import mcap' followed by 'mcap.reader.make_reader(f)' to
'from mcap.reader import make_reader' for cleaner, more explicit import.

This follows Python best practices for explicit imports and improves
compatibility with different mcap package versions.

Signed-off-by: soffer-anyscale <[email protected]>
@soffer-anyscale soffer-anyscale requested a review from a team as a code owner October 21, 2025 21:39
@soffer-anyscale soffer-anyscale changed the title Fix MCAP datasource import for better compatibility [Data] Fix MCAP datasource import for better compatibility Oct 21, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the import statement for the mcap library within the MCAPDatasource to use a more specific import. This is a good change that improves code clarity and explicitness, aligning with Python best practices. The change is well-contained, low-risk, and correctly implemented. I have no further suggestions.

@richardliaw richardliaw added data Ray Data-related issues go add ONLY when ready to merge, run all tests labels Oct 22, 2025
@richardliaw richardliaw merged commit 4ec817b into master Oct 23, 2025
6 checks passed
@richardliaw richardliaw deleted the fix-mcap-reader-import branch October 23, 2025 23:31
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 27, 2025
…ct#57964)

## Description

This PR fixes the import statement in `MCAPDatasource` to follow Python
best practices for explicit imports.

**Change**: Updated from `import mcap` + `mcap.reader.make_reader(f)` to
`from mcap.reader import make_reader` for cleaner, more explicit
importing.

**Why this is needed**:
- Follows Python best practices for explicit imports (PEP 8)
- Improves code readability by making it clear what's being used
- Better compatibility with different mcap package versions
- Reduces namespace pollution

## Related issues

None - this is a code quality improvement.

## Additional information

### What changed

**Before:**
```python
import mcap

reader = mcap.reader.make_reader(f)
```

**After:**
```python
from mcap.reader import make_reader

reader = make_reader(f)
```

### Impact

- **Minimal risk**: This is a refactoring of the import statement only
- **No API changes**: The `MCAPDatasource` public API remains unchanged
- **No behavioral changes**: The functionality is identical
- **Tested**: All ruff lint checks pass

### File modified

- `python/ray/data/_internal/datasource/mcap_datasource.py` (2 lines
changed)

This change makes the code more maintainable and follows modern Python
conventions for explicit imports.

Signed-off-by: soffer-anyscale <[email protected]>
Signed-off-by: xgui <[email protected]>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
…ct#57964)

## Description

This PR fixes the import statement in `MCAPDatasource` to follow Python
best practices for explicit imports.

**Change**: Updated from `import mcap` + `mcap.reader.make_reader(f)` to
`from mcap.reader import make_reader` for cleaner, more explicit
importing.

**Why this is needed**: 
- Follows Python best practices for explicit imports (PEP 8)
- Improves code readability by making it clear what's being used
- Better compatibility with different mcap package versions
- Reduces namespace pollution

## Related issues

None - this is a code quality improvement.

## Additional information

### What changed

**Before:**
```python
import mcap

reader = mcap.reader.make_reader(f)
```

**After:**
```python
from mcap.reader import make_reader

reader = make_reader(f)
```

### Impact

- **Minimal risk**: This is a refactoring of the import statement only
- **No API changes**: The `MCAPDatasource` public API remains unchanged
- **No behavioral changes**: The functionality is identical
- **Tested**: All ruff lint checks pass

### File modified

- `python/ray/data/_internal/datasource/mcap_datasource.py` (2 lines
changed)

This change makes the code more maintainable and follows modern Python
conventions for explicit imports.

Signed-off-by: soffer-anyscale <[email protected]>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
…ct#57964)

## Description

This PR fixes the import statement in `MCAPDatasource` to follow Python
best practices for explicit imports.

**Change**: Updated from `import mcap` + `mcap.reader.make_reader(f)` to
`from mcap.reader import make_reader` for cleaner, more explicit
importing.

**Why this is needed**:
- Follows Python best practices for explicit imports (PEP 8)
- Improves code readability by making it clear what's being used
- Better compatibility with different mcap package versions
- Reduces namespace pollution

## Related issues

None - this is a code quality improvement.

## Additional information

### What changed

**Before:**
```python
import mcap

reader = mcap.reader.make_reader(f)
```

**After:**
```python
from mcap.reader import make_reader

reader = make_reader(f)
```

### Impact

- **Minimal risk**: This is a refactoring of the import statement only
- **No API changes**: The `MCAPDatasource` public API remains unchanged
- **No behavioral changes**: The functionality is identical
- **Tested**: All ruff lint checks pass

### File modified

- `python/ray/data/_internal/datasource/mcap_datasource.py` (2 lines
changed)

This change makes the code more maintainable and follows modern Python
conventions for explicit imports.

Signed-off-by: soffer-anyscale <[email protected]>
Signed-off-by: Aydin Abiar <[email protected]>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
…ct#57964)

## Description

This PR fixes the import statement in `MCAPDatasource` to follow Python
best practices for explicit imports.

**Change**: Updated from `import mcap` + `mcap.reader.make_reader(f)` to
`from mcap.reader import make_reader` for cleaner, more explicit
importing.

**Why this is needed**:
- Follows Python best practices for explicit imports (PEP 8)
- Improves code readability by making it clear what's being used
- Better compatibility with different mcap package versions
- Reduces namespace pollution

## Related issues

None - this is a code quality improvement.

## Additional information

### What changed

**Before:**
```python
import mcap

reader = mcap.reader.make_reader(f)
```

**After:**
```python
from mcap.reader import make_reader

reader = make_reader(f)
```

### Impact

- **Minimal risk**: This is a refactoring of the import statement only
- **No API changes**: The `MCAPDatasource` public API remains unchanged
- **No behavioral changes**: The functionality is identical
- **Tested**: All ruff lint checks pass

### File modified

- `python/ray/data/_internal/datasource/mcap_datasource.py` (2 lines
changed)

This change makes the code more maintainable and follows modern Python
conventions for explicit imports.

Signed-off-by: soffer-anyscale <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants