Skip to content

cli to replace python main_out.py#740

Merged
smilesun merged 5 commits intomasterfrom
issue_374
Jan 19, 2024
Merged

cli to replace python main_out.py#740
smilesun merged 5 commits intomasterfrom
issue_374

Conversation

@agisga
Copy link
Copy Markdown
Collaborator

@agisga agisga commented Jan 11, 2024

Addresses #734

Seems to work fine. I have tested this commit as follows:

poetry build
pip install domainlab-0.4.3-py3-none-any.whl
domainlab -c ./examples/conf/vlcs_diva_mldg_dial.yaml

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 11, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d980abf) 95.12% compared to head (b8f9d63) 95.10%.

Files Patch % Lines
domainlab/cli.py 85.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #740      +/-   ##
==========================================
- Coverage   95.12%   95.10%   -0.03%     
==========================================
  Files         125      126       +1     
  Lines        4906     4920      +14     
==========================================
+ Hits         4667     4679      +12     
- Misses        239      241       +2     
Flag Coverage Δ
unittests 95.10% <85.71%> (-0.03%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@smilesun
Copy link
Copy Markdown
Collaborator

Addresses #734

Seems to work fine. I have tested this commit as follows:

poetry build
pip install domainlab-0.4.3-py3-none-any.whl
domainlab -c ./examples/conf/vlcs_diva_mldg_dial.yaml

nice work! Thanks! Is there a way also to let the pytest cover this new file? Since without testing it will drag the coverage to be below 95% @agisga

@agisga
Copy link
Copy Markdown
Collaborator Author

agisga commented Jan 11, 2024

I'll look into how to include this in pytest.

@agisga
Copy link
Copy Markdown
Collaborator Author

agisga commented Jan 11, 2024

@smilesun I have added some tests. At least on my machine they are passing.

@smilesun
Copy link
Copy Markdown
Collaborator

Do you receive email that CI is broken for this PR? @agisga

@agisga
Copy link
Copy Markdown
Collaborator Author

agisga commented Jan 13, 2024

Do you receive email that CI is broken for this PR? @agisga

Yes, sorry. I think this is because I added the entrypoint via poetry, but CI installed domainlab from setup.py and so the entrypoint isn't created for the domainlab in CI. Should I try to change the CI workflow to use Poetry?

@smilesun
Copy link
Copy Markdown
Collaborator

=================================== FAILURES ===================================
_______________________________ test_entry_point _______________________________

self = [EntryPoint(name='markdown-it', value='markdown_it.cli.parse:main', group='console_scripts'), EntryPoint(name='tqdm', ...f2py.f2py2e:main', group='console_scripts'), EntryPoint(name='gdown', value='gdown.cli:main', group='console_scripts')]
name = 'domainlab'

    def __getitem__(self, name):  # -> EntryPoint:
        """
        Get the EntryPoint in self matching name.
        """
        if isinstance(name, int):
            warnings.warn(
                "Accessing entry points by index is deprecated. "
                "Cast to tuple if needed.",
                DeprecationWarning,
                stacklevel=2,
            )
            return super().__getitem__(name)
        try:
>           return next(iter(self.select(name=name)))
E           StopIteration

/opt/hostedtoolcache/Python/3.10.7/x[64](https://github.com/marrlab/DomainLab/actions/runs/7494347048/job/20402067119?pr=740#step:6:65)/lib/python3.10/importlib/metadata/__init__.py:350: StopIteration

During handling of the above exception, another exception occurred:

    def test_entry_point():
        eps = entry_points()
>       cli_entry = eps.select(group='console_scripts')['domainlab']

tests/test_cli.py:8: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = [EntryPoint(name='markdown-it', value='markdown_it.cli.parse:main', group='console_scripts'), EntryPoint(name='tqdm', ...f2py.f2py2e:main', group='console_scripts'), EntryPoint(name='gdown', value='gdown.cli:main', group='console_scripts')]
name = 'domainlab'

    def __getitem__(self, name):  # -> EntryPoint:
        """
        Get the EntryPoint in self matching name.
        """
        if isinstance(name, int):
            warnings.warn(
                "Accessing entry points by index is deprecated. "
                "Cast to tuple if needed.",
                DeprecationWarning,
                stacklevel=2,
            )
            return super().__getitem__(name)
        try:
            return next(iter(self.select(name=name)))
        except StopIteration:
>           raise KeyError(name)
E           KeyError: 'domainlab'

/opt/hostedtoolcache/Python/3.10.7/x64/lib/python3.10/importlib/metadata/__init__.py:352: KeyError

@smilesun
Copy link
Copy Markdown
Collaborator

Do you receive email that CI is broken for this PR? @agisga

Yes, sorry. I think this is because I added the entrypoint via poetry, but CI installed domainlab from setup.py and so the entrypoint isn't created for the domainlab in CI. Should I try to change the CI workflow to use Poetry?

then I do not have to at you each time. I posted the error above, what you suggested is related to
#739
if changing to poetry could resolve both issues then fine

@smilesun
Copy link
Copy Markdown
Collaborator

will this only be resolved on Thursday?

@agisga
Copy link
Copy Markdown
Collaborator Author

agisga commented Jan 17, 2024

will this only be resolved on Thursday?

I'll try to get it done on Thursday. Sorry, I have a major deadline at work, so didn't really have time to look at GitHub. But tomorrow (Thursday) should be manageable.

@agisga
Copy link
Copy Markdown
Collaborator Author

agisga commented Jan 19, 2024

@smilesun I fixed all checks here by having pytest executed through Poetry (see 2309866 for how Poetry was added to CI). Let me know if there are any other issues/questions.

@smilesun
Copy link
Copy Markdown
Collaborator

@agisga , cool, great work, thanks!

@smilesun smilesun merged commit cf30fa0 into master Jan 19, 2024
@smilesun smilesun deleted the issue_374 branch January 19, 2024 10:10
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.

3 participants