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

Generic CSV views #76

Merged
merged 26 commits into from
Jul 1, 2022
Merged

Generic CSV views #76

merged 26 commits into from
Jul 1, 2022

Conversation

DivineDominion
Copy link
Contributor

CSV previously sported multiple variants and ways to access data that were supposed to be mutually exclusive. With this change, it now really is, ensured by the compiler.

@codecov-io
Copy link

codecov-io commented Apr 26, 2019

Codecov Report

Merging #76 into master will decrease coverage by 0.83%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   92.57%   91.74%   -0.84%     
==========================================
  Files          14       13       -1     
  Lines         431      424       -7     
==========================================
- Hits          399      389      -10     
- Misses         32       35       +3
Impacted Files Coverage Δ
SwiftCSVTests/QuotedTests.swift 100% <100%> (ø) ⬆️
SwiftCSVTests/PerformanceTest.swift 100% <100%> (ø) ⬆️
SwiftCSVTests/EnumeratedViewTests.swift 88.57% <100%> (ø) ⬆️
SwiftCSVTests/TSVTests.swift 100% <100%> (ø) ⬆️
SwiftCSV/NamedView.swift 100% <100%> (ø) ⬆️
SwiftCSVTests/CSVTests.swift 100% <100%> (ø) ⬆️
SwiftCSVTests/URLTests.swift 100% <100%> (ø) ⬆️
SwiftCSV/EnumeratedView.swift 70.83% <22.22%> (-29.17%) ⬇️
SwiftCSV/CSV.swift 70.37% <91.66%> (-1.73%) ⬇️

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 22dc4dd...c5738a9. Read the comment docs.

@DivineDominion
Copy link
Contributor Author

Hmm I haven't had anyone else review this code yet; @lukestringer90 with your real-world application of SwiftCSV, what do you think about the separation of the numbered/named access to CSV from this branch?

@lukestringer90
Copy link
Contributor

@DivineDominion I'll try and take a look for you when I'm next working on my project. Where is the best place to start looking to understand the changes you've made here?

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

Merging #76 (a8af063) into master (1357bfc) will increase coverage by 1.17%.
The diff coverage is 97.82%.

❗ Current head a8af063 differs from pull request most recent head 2badb26. Consider uploading reports for the commit 2badb26 to get more accurate results

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   92.53%   93.70%   +1.17%     
==========================================
  Files          14       14              
  Lines         402      445      +43     
==========================================
+ Hits          372      417      +45     
+ Misses         30       28       -2     
Impacted Files Coverage Δ
SwiftCSV/CSV.swift 66.66% <81.25%> (-4.77%) ⬇️
SwiftCSV/EnumeratedView.swift 100.00% <100.00%> (ø)
SwiftCSV/NamedView.swift 100.00% <100.00%> (ø)
SwiftCSV/String+Lines.swift 44.44% <100.00%> (+15.87%) ⬆️
SwiftCSVTests/CSVTests.swift 100.00% <100.00%> (ø)
SwiftCSVTests/EnumeratedViewTests.swift 90.47% <100.00%> (+4.76%) ⬆️
SwiftCSVTests/NewlineTests.swift 100.00% <100.00%> (ø)
SwiftCSVTests/PerformanceTest.swift 100.00% <100.00%> (ø)
SwiftCSVTests/QuotedTests.swift 100.00% <100.00%> (ø)
SwiftCSVTests/TSVTests.swift 100.00% <100.00%> (ø)
... and 1 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 7461683...2badb26. Read the comment docs.

@DivineDominion
Copy link
Contributor Author

@lukestringer90 I updated the PR to incorporate the v0.5.6 changes.

Offers ways to interact with a CSV via enumeration and also via key/value-like access. The tests should be indicative what's going on behind the scenes. It's mostly a refactoring of the old code that used to mix things up but then errored-out when you used the wrong combination of initial configuration and subsequent data access.

The main question I have is: does this improve the clarity of use and the overall features of the library? Is this a good, or e.g. an over-engineered way forward?

@DivineDominion DivineDominion merged commit 6a1950a into master Jul 1, 2022
@DivineDominion DivineDominion deleted the generic-views branch July 1, 2022 21:13
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.

4 participants