Skip to content
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

@pypi/@conda parsers ... #2306

Merged
merged 7 commits into from
Feb 27, 2025
Merged

@pypi/@conda parsers ... #2306

merged 7 commits into from
Feb 27, 2025

Conversation

savingoyal
Copy link
Collaborator

@savingoyal savingoyal commented Feb 26, 2025

... for requirements.txt, conda.yaml, pyproject.toml

from metaflow import Config, pypi, FlowSpec, requirements_txt_parser, step

class MyFlow(FlowSpec):
    reqs = Config("reqs", default="requirements.txt", parser=requirements_txt_parser)

    @pypi(**reqs)
    @step
    def start(self):
        self.next(self.end)

    @step
    def end(self):
        pass

if __name__ == "__main__":
    MyFlow()

try:
req = Requirement(line)
except InvalidRequirement:
raise ValueError(f"Not a valid PEP 508 requirement: '{line}'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

stacked PR Introduces a MetaflowException type exception for these instead, in order to get cleaner formatting.

Is there any specific reason to stick with ValueError? from what I can tell the exception bubbles all the way and is not caught, leading to a full stack trace in stdout

line = inline_comment_pattern.sub("", line).strip()
if not line:
continue

Copy link
Collaborator

Choose a reason for hiding this comment

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

the previous PR had more specific line handling for requirements.txt content, specifically errors for options:
https://pip.pypa.io/en/stable/reference/requirements-file-format/#global-options

should we reintroduce these, or do we want to keep this implementation as simple as possible, and simply error out on 'unparseable' lines in the file? right now the parser throws an exception if the requirements.txt contains any options, be it global or per-requirement


inside_dependencies = False

# Basic pattern for lines like "numpy=1.21.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

parser breaks with dependency list where there is whitespace between pkg and version, e.g.

dependencies:
  - python = 3.10.*

* change from ValueError to a better formatted metaflow specific error for parsers

* support Rye by skipping over non-requirement lines in lockfile

* add unit tests for parsers.

* fix yml parser regex for whitespaces
@savingoyal savingoyal merged commit 8e719c7 into master Feb 27, 2025
28 checks passed
@savingoyal savingoyal deleted the savin/parsers branch February 27, 2025 16:44
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