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

Fixing new.py, lots of other minor fixes, and lots of tests. #90

Merged
merged 24 commits into from
Jun 7, 2021

Conversation

rbeyer
Copy link
Member

@rbeyer rbeyer commented Jun 1, 2021

  • Attempting to import pvl.new without multidict being available, will now properly yield an ImportError.
  • The dump() and dumps() functions now properly overwritten in pvl.new.
  • All encoders that descended from PVLEncoder didn't properly have group_class and object_class arguments to their constructors, now they do.
  • The char_allowed() function in grammar objects now raises a more useful ValueError than just a generic Exception.
  • The new collections.PVLMultiDict wasn't correctly inserting Mapping objects with the insert_before() and insert_after() methods.
  • The token.Token class's __index__() function didn't always properly return an index.
  • The token.Token class's __float__() function would return int objects if the token could be converted to int. Now always returns floats.
  • So many tests, increased coverage by a lot.

Related Issue

Would close #89

How Has This Been Tested?

  • make lint
  • make docs
  • make test-all

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Licensing:

This project is released under the LICENSE.

  • I claim copyrights on my contributions in this pull request, and I provide those contributions via this pull request under the same license terms that the pvl project uses.

rbeyer added 24 commits May 31, 2021 19:43
…be imported by a user without multidict being installed.
…ually returns floats instead of sometimes ints, and some documentation fixed.
… otherwise get ImportError, even though not required for normal install.
…t conversion, otherwise python38 correctly complains about a race condition.
@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #90 (fa6f196) into main (4c4f691) will increase coverage by 14.99%.
The diff coverage is 90.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main      #90       +/-   ##
===========================================
+ Coverage   79.85%   94.84%   +14.99%     
===========================================
  Files          12       12               
  Lines        1911     1922       +11     
===========================================
+ Hits         1526     1823      +297     
+ Misses        385       99      -286     
Impacted Files Coverage Δ
pvl/encoder.py 89.24% <ø> (+1.22%) ⬆️
pvl/new.py 86.00% <83.33%> (+86.00%) ⬆️
pvl/__init__.py 94.73% <100.00%> (ø)
pvl/collections.py 98.70% <100.00%> (+27.67%) ⬆️
pvl/grammar.py 100.00% <100.00%> (+3.88%) ⬆️
pvl/pvl_translate.py 100.00% <100.00%> (+100.00%) ⬆️
pvl/pvl_validate.py 99.06% <100.00%> (+99.06%) ⬆️
pvl/token.py 100.00% <100.00%> (+10.71%) ⬆️
pvl/lexer.py 100.00% <0.00%> (+0.76%) ⬆️
... and 9 more

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 4c4f691...fa6f196. Read the comment docs.

@rbeyer rbeyer merged commit d2c965c into planetarypy:main Jun 7, 2021
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.

pvl.new not quite working
1 participant