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

acousticbrainz: Really small float values are stored as strings #2790

Closed
Kernald opened this issue Jan 13, 2018 · 10 comments · Fixed by #3238
Closed

acousticbrainz: Really small float values are stored as strings #2790

Kernald opened this issue Jan 13, 2018 · 10 comments · Fixed by #3238
Labels
bug bugs that are confirmed and actionable

Comments

@Kernald
Copy link
Contributor

Kernald commented Jan 13, 2018

Problem

Using the acousticbrainz plugin (I don't know if it's specific to it, it might be a wider issue), when storing really small values, they end up formatted in an exponential style, e.g. 1.82764233614e-05. As a result, the whole (flexible, in this case) attribute ends up being treated as a string - which forbids using ranges in query, for example.

Here's an example with the recording 9dcdb20b-3bdf-4d1f-842f-d3cb545bea28 on MusicBrainz. The mood_happy probability is really low on AcousticBrainz: http://acousticbrainz.org/api/v1/9dcdb20b-3bdf-4d1f-842f-d3cb545bea28/high-level?n=0 (field value is 0.0000182764233614). In beets, it ends up being stored as 1.82764233614e-05:

$ beet -vv acousticbrainz M83 Ludivine -f
user configuration: /root/.config/beets/config.yaml
data directory: /root/.config/beets
plugin paths: /root/.config/beets/beets-alternatives/beetsplug
Sending event: pluginload
lyrics: Disabling google source: no API key configured.
inline: adding item field disc_with_title
inline: adding album field spinnin_catalognum
library database: /root/.config/beets/beets.blb
library directory: /media/sao/Musique
Sending event: library_opened
acousticbrainz: getting data for: M83 - Junk - Ludivine
acousticbrainz: fetching URL: https://acousticbrainz.org/9dcdb20b-3bdf-4d1f-842f-d3cb545bea28/low-level
acousticbrainz: fetching URL: https://acousticbrainz.org/9dcdb20b-3bdf-4d1f-842f-d3cb545bea28/high-level
acousticbrainz: attribute danceable of M83 - Junk - Ludivine set to 0.00766881043091
acousticbrainz: attribute gender of M83 - Junk - Ludivine set to male
acousticbrainz: attribute genre_rosamerica of M83 - Junk - Ludivine set to cla
acousticbrainz: attribute mood_acoustic of M83 - Junk - Ludivine set to 0.995353162289
acousticbrainz: attribute mood_aggressive of M83 - Junk - Ludivine set to 0.042748786509
acousticbrainz: attribute mood_electronic of M83 - Junk - Ludivine set to 0.0935237780213
acousticbrainz: attribute mood_happy of M83 - Junk - Ludivine set to 1.82764233614e-05
acousticbrainz: attribute mood_party of M83 - Junk - Ludivine set to 0.0153690651059
acousticbrainz: attribute mood_relaxed of M83 - Junk - Ludivine set to 0.987963736057
acousticbrainz: attribute mood_sad of M83 - Junk - Ludivine set to 0.956809282303
acousticbrainz: attribute rhythm of M83 - Junk - Ludivine set to Tango
acousticbrainz: attribute tonal of M83 - Junk - Ludivine set to 0.435981780291
acousticbrainz: attribute voice_instrumental of M83 - Junk - Ludivine set to instrumental
acousticbrainz: attribute average_loudness of M83 - Junk - Ludivine set to 0.753472864628
acousticbrainz: attribute bpm of M83 - Junk - Ludivine set to 106.67628479
acousticbrainz: attribute chords_changes_rate of M83 - Junk - Ludivine set to 0.0346172600985
acousticbrainz: attribute chords_key of M83 - Junk - Ludivine set to D
acousticbrainz: attribute chords_number_rate of M83 - Junk - Ludivine set to 0.00487567018718
acousticbrainz: attribute chords_scale of M83 - Junk - Ludivine set to major
acousticbrainz: attribute key_strength of M83 - Junk - Ludivine set to 0.704598069191
acousticbrainz: attribute initial_key of M83 - Junk - Ludivine set to F# minor
Sending event: database_change
Sending event: write

Setup

  • OS: Linux
  • Python version: 3.6.4
  • beets version: 1.4.6
  • Turning off plugins made problem go away (yes/no): irrelevant, I guess

My configuration (output of beet config) is: probably irrelevant too, I just have the plugin enabled without any configuration.

@sampsyo
Copy link
Member

sampsyo commented Jan 13, 2018

Wow, that’s pretty bad! Is it possible to cause the same problem using beet modify to set a value to a very small number? If so, we’ll know this is an issue in the core, not with this specific plugin.

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Jan 13, 2018
@Kernald
Copy link
Contributor Author

Kernald commented Jan 13, 2018

Good catch. It works fine with beet modify (either in the preview and in the saved value, displayed with beet ls.

@sampsyo
Copy link
Member

sampsyo commented Jan 13, 2018

Thanks! It looks like we need to be more careful about how we convert values from the analysis tool’s output to numeric Python values.

@sampsyo sampsyo changed the title Really small float values are stored as strings acousticbrainz: Really small float values are stored as strings Jan 13, 2018
@sampsyo sampsyo added bug bugs that are confirmed and actionable and removed needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." labels Jan 13, 2018
@rain0r
Copy link
Contributor

rain0r commented Apr 22, 2019

@sampsyo

Could this be the solution?
#3233

@sampsyo
Copy link
Member

sampsyo commented Apr 22, 2019

Good digging, @rain0r. This reveals a slightly deeper problem, however: your change converts the value to be stored as a string, but we probably just want to store the underlying floating-point number! I believe the current code (without your change) is also converting to a string for storage, but in a less careful way that results in scientific notation.

A better thing would be to take all the relevant fields and mark them as having a float type. To try this out, you could try enabling the types plugin and marking fields like danceable as having type float. Then the storage from the AcousticBrainz plugin should work correctly. If that does indeed solve the problem, then we should augment the AB plugin to declare these types for all relevant fields, like mpdstats does:

beets/beetsplug/mpdstats.py

Lines 315 to 320 in 1edd21f

item_types = {
'play_count': types.INTEGER,
'skip_count': types.INTEGER,
'last_played': library.DateType(),
'rating': types.FLOAT,
}

@rain0r
Copy link
Contributor

rain0r commented Apr 23, 2019

Thanks for the tip, @sampsyo, unfortunately, I didn't get it to work.
Please see my latest commit.
With mood_happy as types.FLOAT the result is '0.0 (for this song).
With the type types.STRING the result is 1.82764233614e-05'.

@sampsyo
Copy link
Member

sampsyo commented Apr 23, 2019

Aha—any chance the correct value is being stored, but it's just being rounded when it's printed to the console? The stringification for floats uses a fixed precision:
https://github.com/beetbox/beets/blob/master/beets/dbcore/types.py#L183

Perhaps we could make the precision a parameter to the type, so this field could use more than one decimal place when printing to the console.

@rain0r
Copy link
Contributor

rain0r commented Apr 23, 2019

You are right, with a bigger precision, the displayed value is correct.
Would you prefer the parameter-solution or just increasing the precision?

@sampsyo
Copy link
Member

sampsyo commented Apr 23, 2019

Nice! Given that there are other float-valued fields in the system that probably want only 1 digit of precision (e.g., the ReplayGain fields), I think adding a precision parameter would be best. We can use a similar approach to the PaddedInt type:

beets/beets/dbcore/types.py

Lines 135 to 143 in 4d55e6d

class PaddedInt(Integer):
"""An integer field that is formatted with a given number of digits,
padded with zeroes.
"""
def __init__(self, digits):
self.digits = digits
def format(self, value):
return u'{0:0{1}d}'.format(value or 0, self.digits)

@rain0r
Copy link
Contributor

rain0r commented Apr 24, 2019

Sooo @sampsyo, I created (or rather, renamed) a new branch because I had problems checking it out locally.

Please see: #3238

Thanks for your help!

sampsyo added a commit that referenced this issue Apr 30, 2019
Fix for #2790 - acousticbrainz: Really small float values are stored as strings
sampsyo added a commit that referenced this issue Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs that are confirmed and actionable
Projects
None yet
3 participants