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

Inherit Roo::Excelx::Coordinate from Array #458

Merged
merged 1 commit into from
Sep 21, 2018

Conversation

chopraanmol1
Copy link
Member

Profiling Script

MemoryProfiler.report{ Roo::Excelx.new(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }.pretty_print

Master:

Total allocated: 34559100 bytes (504994 objects)
Total retained:  5563403 bytes (103022 objects)

allocated memory by gem
-----------------------------------
  19254338  roo/lib
   8509100  nokogiri-1.8.4
   6793822  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    358381  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   5561094  roo/lib
       792  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
    102989  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other

After Patch:

Total allocated: 33439850 bytes (477013 objects)
Total retained:  4444153 bytes (75041 objects)

allocated memory by gem
-----------------------------------
  18135098  roo/lib
   8509100  nokogiri-1.8.4
   6793812  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    330400  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   4441854  roo/lib
       782  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
     75008  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other

Summary

Provide a general description of the code changes in your pull
request... were there any bugs you had fixed? If so, mention them. If
these bugs have open GitHub issues, be sure to tag them here as well,
to keep the conversation linked together.

Other Information

If there's anything else that's important and relevant to your pull
request, mention that information here. This could include
benchmarks, or other information.

Thanks for contributing to Roo!

@coveralls
Copy link

coveralls commented Sep 20, 2018

Coverage Status

Coverage decreased (-0.01%) to 93.836% when pulling 00356c2 on chopraanmol1:inherit_coordinate_from_array into bfcfba4 on roo-rb:master.

@tgturner
Copy link
Contributor

This looks good! Might be worth adding some basic test coverage for the Coordinate class and for extract_coordinates

Profiling Script
```
MemoryProfiler.report{ Roo::Excelx.new(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }.pretty_print
```

Master:
```
Total allocated: 34559100 bytes (504994 objects)
Total retained:  5563403 bytes (103022 objects)

allocated memory by gem
-----------------------------------
  19254338  roo/lib
   8509100  nokogiri-1.8.4
   6793822  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    358381  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   5561094  roo/lib
       792  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
    102989  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other
```

After Patch:
```
Total allocated: 33439850 bytes (477013 objects)
Total retained:  4444153 bytes (75041 objects)

allocated memory by gem
-----------------------------------
  18135098  roo/lib
   8509100  nokogiri-1.8.4
   6793812  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    330400  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   4441854  roo/lib
       782  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
     75008  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other
```
@chopraanmol1
Copy link
Member Author

@tgturner as per your suggestion I've added test coverage for extract_coordinate and Coordinate class

@tgturner tgturner merged commit 6eb877e into roo-rb:master Sep 21, 2018
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)
aravindm pushed a commit to chobiwa/roo that referenced this pull request Jun 18, 2019
Profiling Script
```
MemoryProfiler.report{ Roo::Excelx.new(file_name).tap{|x|(2..x.last_row).each{|i| x.row(i)}} }.pretty_print
```

Master:
```
Total allocated: 34559100 bytes (504994 objects)
Total retained:  5563403 bytes (103022 objects)

allocated memory by gem
-----------------------------------
  19254338  roo/lib
   8509100  nokogiri-1.8.4
   6793822  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    358381  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   5561094  roo/lib
       792  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
    102989  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other
```

After Patch:
```
Total allocated: 33439850 bytes (477013 objects)
Total retained:  4444153 bytes (75041 objects)

allocated memory by gem
-----------------------------------
  18135098  roo/lib
   8509100  nokogiri-1.8.4
   6793812  rubyzip-1.2.2
      1304  tmpdir
       320  weakref
       216  other

allocated objects by gem
-----------------------------------
    330400  roo/lib
    145846  nokogiri-1.8.4
       735  rubyzip-1.2.2
        22  tmpdir
         8  weakref
         2  other

retained memory by gem
-----------------------------------
   4441854  roo/lib
       782  rubyzip-1.2.2
       725  nokogiri-1.8.4
       320  weakref
       296  tmpdir
       176  other

retained objects by gem
-----------------------------------
     75008  roo/lib
        14  nokogiri-1.8.4
         8  weakref
         7  rubyzip-1.2.2
         3  tmpdir
         1  other
```
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