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

Restore support of URL for CSV #462

Merged

Conversation

chopraanmol1
Copy link
Member

@chopraanmol1 chopraanmol1 commented Oct 5, 2018

Summary

Fixes #378 #374 (Introduced in #368)

Benchmark

file_name = 'test/files/Bibelbund.csv'

MemoryProfiler.report{ Roo::Spreadsheet.open(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }
puts Benchmark.measure{ Roo::Spreadsheet.open(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }

Master

Total allocated: 39705265 bytes (561479 objects)
Total retained:  768 bytes (4 objects)

  0.300000   0.000000   0.300000 (  0.304877)

Modified:

Total allocated: 16952085 bytes (234487 objects)
Total retained:  768 bytes (4 objects)

  0.190000   0.000000   0.190000 (  0.181199)

@chopraanmol1
Copy link
Member Author

@matiasgarcia can you verify if this PR solves your use case

@coveralls
Copy link

coveralls commented Oct 5, 2018

Coverage Status

Coverage increased (+0.3%) to 94.079% when pulling e29ffe7 on chopraanmol1:restore_support_or_url_for_csv into 8d5ea7a on roo-rb:master.

### Benchmark

```
file_name = 'test/files/Bibelbund.csv'

MemoryProfiler.report{ Roo::Spreadsheet.open(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }
puts Benchmark.measure{ Roo::Spreadsheet.open(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }
```

Master
```
Total allocated: 39705265 bytes (561479 objects)
Total retained:  768 bytes (4 objects)

  0.300000   0.000000   0.300000 (  0.304877)
```

Modified:
```
Total allocated: 16952085 bytes (234487 objects)
Total retained:  768 bytes (4 objects)

  0.190000   0.000000   0.190000 (  0.181199)
```
@roo-rb roo-rb deleted a comment from coveralls Oct 5, 2018
@tgturner
Copy link
Contributor

tgturner commented Oct 6, 2018

I will wait to see if @matiasgarcia can verify that this addresses his issue before merging.

@tgturner
Copy link
Contributor

tgturner commented Oct 7, 2018

@chopraanmol1 Would you like to keep waiting to see if this is confirmed by the user? I'm ok with going ahead and merging if you are.

@chopraanmol1
Copy link
Member Author

@tgturner lets go ahead with this. We can address additional cases (if any) in separate PR.

@chopraanmol1 chopraanmol1 merged commit 4053242 into roo-rb:master Oct 7, 2018
@chopraanmol1 chopraanmol1 deleted the restore_support_or_url_for_csv branch October 7, 2018 18:26
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 20, 2019
pkgsrc change: add "USE_LANGUAGES= # none".

##  [2.8.0] 2019-01-18
### Fixed
- Fixed inconsistent column length for CSV [375](roo-rb/roo#375)
- Fixed formatted_value with `%` for Excelx [416](roo-rb/roo#416)
- Improved Memory consumption and performance [434](roo-rb/roo#434) [449](roo-rb/roo#449) [454](roo-rb/roo#454) [456](roo-rb/roo#456) [458](roo-rb/roo#458) [462](roo-rb/roo#462) [466](roo-rb/roo#466)
- Accept both Transitional and Strict Type for Excelx's worksheets [441](roo-rb/roo#441)
- Fixed ruby warnings [442](roo-rb/roo#442) [476](roo-rb/roo#476)
- Restore support for URL as file identifier for CSV [462](roo-rb/roo#462)
- Fixed missing location for Excelx's links [482](roo-rb/roo#482)

### Changed / Added
- Drop support for ruby 2.2.x and lower
- Updated rubyzip version for fixing security issue. Now minimal version is 1.2.1
- Roo::Excelx::Coordinate now inherits Array [458](roo-rb/roo#458)
- Improved Roo::HeaderRowNotFoundError exception's message [461](roo-rb/roo#461)
- Added `empty_cell` option which by default disable allocation for Roo::Excelx::Cell::Empty [464](roo-rb/roo#464)
- Added support for variable number of decimals for Excelx's formatted_value [387](roo-rb/roo#387)
- Added `disable_html_injection` option to disable html injection for shared string in `Roo::Excelx` [392](roo-rb/roo#392)
- Added image extraction for Excelx [414](roo-rb/roo#414) [397](roo-rb/roo#397)
- Added support for `1e6` as scientific notation for Excelx [433](roo-rb/roo#433)
- Added support for Integer as 0 based index for Excelx's `sheet_for` [455](roo-rb/roo#455)
- Extended `no_hyperlinks` option for non streaming Excelx methods [459](roo-rb/roo#459)
- Added `empty_cell` option to disable Roo::Excelx::Cell::Empty allocation for Excelx [464](roo-rb/roo#464)
- Added support for Integer with leading zero for Roo:Excelx [479](roo-rb/roo#479)
- Refactored Excelx code [453](roo-rb/roo#453) [477](roo-rb/roo#477) [483](roo-rb/roo#483) [484](roo-rb/roo#484)

### Deprecations
- Roo::Excelx::Sheet#present_cells is deprecated [454](roo-rb/roo#454)
- Roo::Utils.split_coordinate is deprecated [458](roo-rb/roo#458)
- Roo::Excelx::Cell::Base#link is deprecated [457](roo-rb/roo#457)
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