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.

Sign up for GitHub

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

Jump to bottom

Add Support for JSONL Writer #628

Closed
wants to merge 11 commits into from
Closed

Add Support for JSONL Writer #628

wants to merge 11 commits into from

Conversation

mzaeemz
Copy link
Contributor

@mzaeemz mzaeemz commented Sep 23, 2022

This PR has the objective of adding support for writing to JSONL files

Changes

  1. Updated tojson function in petl.io.json to write in JSONL format if lines argument is set to true

Checklist

Use this checklist for assuring the quality of pull requests that include new code and or make changes to existing code.

  • Source Code rules apply:
    • Includes unit tests
    • New functions have docstrings with examples that can be run with doctest
    • New functions are included in API docs
    • Docstrings include notes for any changes to API or behaviour
    • All changes are documented in docs/changes.rst
  • Versioning and history tracking rules apply:
    • Using atomic commits when possible
    • Commits are reversible when possible
    • There is no incomplete changes in the pull request
    • There is no accidental garbage added in source code
  • Testing rules apply:
    • Tested locally using tox / pytest
    • Automated testing passes (see CI)
    • Unit test coverage has not decreased (see Coveralls)
  • State of these changes is:
    • Just a proof of concept
    • Work in progress / Further changes needed
    • Ready to review
    • Ready to merge

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3111981167

  • 42 of 43 (97.67%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 91.117%
Changes Missing Coverage Covered Lines Changed/Added Lines %
petl/io/json.py 10 11 90.91%
Totals Coverage Status
Change from base Build 2899567235: 0.02%
Covered Lines: 12750
Relevant Lines: 13993
💛 - Coveralls

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 23, 2022
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 16 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 23, 2022
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 12 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 23, 2022
View reviewed changes
petl/io/json.py
... print(open('example.file3.jsonl').read())
{"name": "Gilbert", "wins": [["straight", "7S"], ["one pair", "10H"]]}
{"name": "Alexa", "wins": [["two pair", "4S"], ["two pair", "9S"]]}
{"name": "May", "wins": []}

Check warning

Code scanning / Pylint (reported by Codacy)

Trailing whitespace

Trailing whitespace
petl/io/json.py
{"name": "Alexa", "wins": [["two pair", "4S"], ["two pair", "9S"]]}
{"name": "May", "wins": []}
{"name": "Deloise", "wins": [["three of a kind", "5S"]]}

Check warning

Code scanning / Pylint (reported by Codacy)

Trailing whitespace

Trailing whitespace
petl/io/json.py
finally:
f.detach()


def _writeobj(encoder, obj, f, prefix, suffix):
def _writeobj(encoder, obj, f, prefix, suffix, lines=False):

Check warning

Code scanning / Pylint (reported by Codacy)

Argument name "f" doesn't conform to snake_case naming style

Argument name "f" doesn't conform to snake_case naming style
petl/io/json.py
>>> table1 = [['name', 'wins'],
... ['Gilbert', [['straight', '7S'], ['one pair', '10H']]],
... ['Alexa', [['two pair', '4S'], ['two pair', '9S']]],
... ['May', []],

Check warning

Code scanning / Pylint (reported by Codacy)

Trailing whitespace

Trailing whitespace
petl/io/json.py
{"name": "May", "wins": []}
{"name": "Deloise", "wins": [["three of a kind", "5S"]]}


Check warning

Code scanning / Pylint (reported by Codacy)

Trailing whitespace

Trailing whitespace
petl/io/json.py
finally:
f.detach()


def _writeobj(encoder, obj, f, prefix, suffix):
def _writeobj(encoder, obj, f, prefix, suffix, lines=False):

Check warning

Code scanning / Pylint (reported by Codacy)

Too many arguments (6/5)

Too many arguments (6/5)
petl/test/io/test_jsonl.py
assert result[2]['bar'] == 2


def test_tojson_2():

Check warning

Code scanning / Pylint (reported by Codacy)

Missing function docstring

Missing function docstring
petl/test/io/test_jsonl.py
@@ -49,3 +50,44 @@

ieq(expect, actual)
ieq(expect, actual) # verify can iterate twice


def test_tojson_1():

Check warning

Code scanning / Pylint (reported by Codacy)

Missing function docstring

Missing function docstring
petl/test/io/test_jsonl.py
['Alexa', [['two pair', '4S'], ['two pair', '9S']]],
['May', []],
['Deloise', [['three of a kind', '5S']]]]
f = NamedTemporaryFile(delete=False, mode='r')

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "f" doesn't conform to snake_case naming style

Variable name "f" doesn't conform to snake_case naming style
petl/test/io/test_jsonl.py
('a', 1),
('b', 2),
('c', 2))
f = NamedTemporaryFile(delete=False, mode='r')

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "f" doesn't conform to snake_case naming style

Variable name "f" doesn't conform to snake_case naming style
@juarezr juarezr added the Feature A nice to have thing that we don't have yet label Sep 25, 2022
juarezr
juarezr requested changes Sep 25, 2022
View reviewed changes
Copy link
Member

@juarezr juarezr left a comment •
edited

Choose a reason for hiding this comment

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

@mzaeemz,

  • Nice feature! Well done. 👍
  • I noticed you flagged the "ready to review" and after reading it, I think it's pretty good.
  • I have just one suggestion:
    • For preserving the code history, it may be better to consolidate the changes in 2 or 3 atomic commits:
      1. .gitignore
      2. json.py
      3. test_jsonl.py (but it's ok if grouped together with the previous)
    • Do you mind making a git rebase -i master for rewriting/squashing the commits?

@mzaeemz mzaeemz closed this Sep 26, 2022
@mzaeemz mzaeemz deleted the f/jsonl-writer-support branch September 26, 2022 09:44
@mzaeemz
Copy link
Contributor Author

mzaeemz commented Sep 26, 2022

@juarezr
Thank you for the feedback. However, my noob self ended up deleting the branch. I have created a new PR here, kindly take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juarezr juarezr juarezr requested changes

Assignees

@mzaeemz mzaeemz

Labels
Feature A nice to have thing that we don't have yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mzaeemz @coveralls @juarezr @Wassey16