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

Fix internal use of deprecated Excelx::Cell.new #564

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cgunther
Copy link

Calling Excelx#set internally called Excelx::Cell.new, which is deprecated in favor of Excelx::Cell.create_cell.

I tried using the recommended Excelx::Cell.create_cell, however it expects a type as the first argument. #set tries to infer the type via cell_type_by_value, however it only returns :float or :string, but Excelx::Cell.cell_class doesn't support :float.

Even if I add :float to map to Cell::Number, then there's a dilemma because Excelx::Cell.create_cell passes all the arguments except the first onto the specific cell class, but the arity of Cell::String is 5 whereas the arity of Cell::Number is 6, meaning Excelx#set would need to initialize each cell class individually to pass the appropriate arguments.

Therefore I landed on simply using Cell::Base. It's probably not the most accurate, but given persisting the spreadsheet isn't an option, the uses for Excelx#set should be minimal. In my case, I simply use it in testing to avoid creating new files for every possible scenario, opting to manually set various cells to triggered assorted scenarios.

Fixes #529.

Calling `Excelx#set` internally called `Excelx::Cell.new`, which is
deprecated in favor of `Excelx::Cell.create_cell`.

I tried using the recommended `Excelx::Cell.create_cell`, however it
expects a `type` as the first argument. `#set` tries to infer the type
via  [`cell_type_by_value`](https://github.com/roo-rb/roo/blob/709464c77623be2bc09b2103405d90ded7604a75/lib/roo/base.rb#L175-L182),
however it only returns `:float` or `:string`, but
`Excelx::Cell.cell_class` doesn't support `:float`.

Even if I add `:float` to map to `Cell::Number`, then there's a dilemma
because `Excelx::Cell.create_cell` passes all the arguments except the
first onto the specific cell class, but the arity of `Cell::String` is 5
whereas the arity of `Cell::Number` is 6, meaning `Excelx#set` would
need to initialize each cell class individually to pass the appropriate
arguments.

Therefore I landed on simply using `Cell::Base`. It's probably not the
most accurate, but given persisting the spreadsheet isn't an option, the
uses for `Excelx#set` should be minimal. In my case, I simply use it in
testing to avoid creating new files for every possible scenario, opting
to manually set various cells to triggered assorted scenarios.

Fixes roo-rb#529.
@gee-forr
Copy link

Is there any chance this PR will get merged and released? Our logs are absolutely spammed by this warning. Alternatively, is there a way we can get it to display only once instead of every time it's called?

@gee-forr
Copy link

Hey all - I know you're all really busy, but is there any chance of merging this in and getting a fresh version of this gem released to rubygems?

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.

Getting deprecated Cell.new, use Cell.create_cell for set() method of excelx.rb file
2 participants