Skip to content

Refactoring of lib.fileio.auto_open function to work with subprocess (#266) #267

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Reovirus
Copy link

  1. CommandFormatter Interface
    Introduced the CommandFormatter interface, encapsulating file operations and validations.​
    Added standard CLI response outputs to enhance user interaction.​
  2. MODES_TO_FILES_PRESET and COMMANDS Configurations (just 2 dicts)
    MODES_TO_FILES_PRESET: format keys with values as lists of possible operation modes (read, write, append).
    COMMANDS: format and mode keys with values as dictionaries containing:
    tool: the tool's presence is verified in the system.
    command: a command formatted for substituting the number of available processors (nproc).
    This structure streamlines adding new formats without altering existing code.​
  3. Binary File Support in CommandFormatter
    Added capabilities for handling binary files.​
    This feature is not currently utilized but provides flexibility for future enhancements.​
  4. Integration with subprocess
    Transitioned to using subprocess for file operations, improving compatibility and performance.​

P.S. The function signatures and responses of auto_open remain unchanged, ensuring backward compatibility. However, the implementation using the CommandFormatter class offers a more convenient and versatile approach, enhancing code maintainability and scalability.​

@Reovirus Reovirus changed the title Refactoring of lib.fileio.auto_open function to work with subprocess Refactoring of lib.fileio.auto_open function to work with subprocess https://github.com/open2c/pairtools/issues/266 Apr 13, 2025
@Reovirus Reovirus changed the title Refactoring of lib.fileio.auto_open function to work with subprocess https://github.com/open2c/pairtools/issues/266 Refactoring of lib.fileio.auto_open function to work with subprocess [266](https://github.com/open2c/pairtools/issues/266) Apr 13, 2025
@Reovirus Reovirus changed the title Refactoring of lib.fileio.auto_open function to work with subprocess [266](https://github.com/open2c/pairtools/issues/266) ​Here's a formatted summary of the pull request titled "Refactoring of lib.fileio.auto_open function to work with subprocess" (#266): Apr 13, 2025
@Reovirus Reovirus changed the title ​Here's a formatted summary of the pull request titled "Refactoring of lib.fileio.auto_open function to work with subprocess" (#266): Refactoring of lib.fileio.auto_open function to work with subprocess (#266) Apr 13, 2025
Reovirus and others added 4 commits April 17, 2025 21:16
is_binary: bool=False

@staticmethod
def form_notfounderror_text(searched_tools: tp.List[str], is_read: bool) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Would be great to add comment message to each method: what it's doing. Very simple description is enough!

raise ValueError(f'{self.__format} can not to be opened in {self.mode}')

checked_tools = []
for possible_solution in COMMANDS[(self.__format, self.mode)]:
Copy link
Member

Choose a reason for hiding this comment

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

Just a brief annotation of the flow of checks will be great (one comment line for each critical point, no more)

'lz4': ['w', 'r', 'a'],
}

COMMANDS = {
Copy link
Member

Choose a reason for hiding this comment

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

Would be great to write down the logic in the comment here: the first command is the first choice, etc.

self.command = possible_solution['command'].format(str(self.nproc))
return

raise ValueError(self.form_notfounderror_text(checked_tools, self.mode=='r'))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe comment line here will be helpful

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