Faker: Deprecation warnings coming from within the library

Created on 10 Jul 2019  路  12Comments  路  Source: faker-ruby/faker

Describe the bug

In version 1.9.6 calling Numer::decimal results in deprecation warnings, because Number::decimal uses Number::decimal_part.

Additionally, there is a bug in the deprecation warning where it is suggesting an empty string as a replacement. (Same steps to reproduce)

NOTE: Faker::Number.decimal_part is deprecated; use  instead. It will be removed on or after 2019-06-01.
Faker::Number.decimal_part called from /usr/local/bundle/gems/faker-1.9.6/lib/faker/default/number.rb:34.
NOTE: Faker::Number.leading_zero_number is deprecated; use  instead. It will be removed on or after 2019-06-01.
Faker::Number.leading_zero_number called from /usr/local/bundle/gems/faker-1.9.6/lib/faker/default/number.rb:29.

To Reproduce

Use Number::decimal anywhere. I found it using rake test, but the easiest repro is with an interactive ruby terminal, e.g. in pry:

[1] pry(main)> require 'faker'
=> true
[2] pry(main)> Faker::Number.decimal
NOTE: Faker::Number.decimal_part is deprecated; use  instead. It will be removed on or after 2019-06-01.
Faker::Number.decimal_part called from /Users/nathanhinchey/.rvm/gems/ruby-2.6.2/gems/faker-1.9.6/lib/faker/default/number.rb:34.
NOTE: Faker::Number.leading_zero_number is deprecated; use  instead. It will be removed on or after 2019-06-01.
Faker::Number.leading_zero_number called from /Users/nathanhinchey/.rvm/gems/ruby-2.6.2/gems/faker-1.9.6/lib/faker/default/number.rb:29.
=> "78132.07"

Expected behavior

There should not be a deprecation warning for code that is not deprecated. Non-deprecated code should not call deprecated code.

Additionally, the deprecation warning should suggest an alternative instead of an empty string.

Most helpful comment

@connorshea it's going to be fixed in this PR https://github.com/stympy/faker/pull/1651. The change is here.

All 12 comments

Possibly related to https://github.com/stympy/faker/issues/1610 "Faker Deprecation Warnings showing up in Rails Console" ?

This issue will be fixed in v2. Thanks for reporting.

This is a little bit frustrating to see happen. I went searching for where I was possibly using Faker incorrectly, but it seems like you are warning yourselves? I can't see some actual CI output currently because I have 100+ lines of these deprecations.

Can this be turned off through a flag? When is v2 releasing if not?

Thanks for our work on the library! Use it all the time.

@rylanb sorry about that! We'll remove all the deprecation warnings in v2.

I can commit myself to get the pending things done this weekend and publish v2.

@vbrazo oh no worries. In the grand scheme of things, not a big deal. I was more concerned that it would be months before a v2 might come out. If its a couple weeks, not a big deal.

Thanks for responding!

Is the solution for now just to wait, or is there a change that should be made if you're using the decimal method?

@connorshea probably the easiest solution for now is to jump back before the deprecation was added (~> 1.9.4?). Have to wait for v2 to come out, I think?

For now I'm just going to hack around it since I only have one call to the decimal method. It isn't pretty, but it does seem to work:

# Before
Faker::Number.decimal(3, 1)
# After
"#{rand(999)}.#{rand(10)}".to_f

Hopefully that's useful to someone while we wait :P

@connorshea it's going to be fixed in this PR https://github.com/stympy/faker/pull/1651. The change is here.

Please feel free to review this PR. I need some eyes on it @rylanb @connorshea @nathanhinchey

@vbrazo I tried to review it but the context is beyond me. I think @nathanhinchey got you covered though!

@rylanb yeah! He did a good job. I still need to read that PR again and apply any necessary changes. I think we're getting pretty close.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tagliala picture tagliala  路  5Comments

ggrillone picture ggrillone  路  6Comments

mkeiser82 picture mkeiser82  路  3Comments

kevgathuku picture kevgathuku  路  4Comments

CJAdeszko picture CJAdeszko  路  6Comments