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

Local import YAML instead of top level #53

Merged
merged 1 commit into from Aug 4, 2022

Conversation

oz123
Copy link
Contributor

@oz123 oz123 commented Jul 25, 2022

Hi! pipenv maintainer here...

We, @matteius and me, are trying to clean up vendored libraries shipped with pipenv.
I noticed that YAML is only used to parse conda files. If you merge in this patch, it would allow us to drop this
dependency.

You can also make YAML and optional dependency for your users with:

setup(
 name='dparse',
    version='0.5.2a',
    description="A parser for Python dependency files",
    long_description=readme + '\n\n' + history,
   ...
    extras_require={
        'pipenv':  ["pipenv"],
        'conda': ["yaml"],
    }
)

However, I didn't submit a patch for this just to limit the scope of this PR.

Thank you very much.

yaml is only used to parse conda files.
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #53 (5a3ad57) into master (a3d9beb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   94.78%   94.80%   +0.01%     
==========================================
  Files          11       11              
  Lines         806      808       +2     
==========================================
+ Hits          764      766       +2     
  Misses         42       42              
Impacted Files Coverage Δ
dparse/parser.py 87.68% <100.00%> (ø)
tests/test_dependencies.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3d9beb...5a3ad57. Read the comment docs.

@yeisonvargasf yeisonvargasf self-requested a review August 4, 2022 19:31
Copy link
Member

@yeisonvargasf yeisonvargasf left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I'll add it as an optional dependency.

@yeisonvargasf yeisonvargasf merged commit 3290bb5 into pyupio:master Aug 4, 2022
@oz123
Copy link
Contributor Author

oz123 commented Aug 4, 2022

Cool! Thanks for merging. Can you also please tag a release?

@yeisonvargasf
Copy link
Member

@oz123 Yes, I'll do that this week. I'm waiting to merge some changes from an advisory, and I'll tag and release it.

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.

None yet

2 participants