ropensci / bibtex

bibtex parser for R
https://docs.ropensci.org/bibtex/
35 stars 12 forks source link

DONT WRITE BACK TO YOUR BIBTEX-FILE: custom fields are imported with column-names that includes the values in the fields...!! #38

Open emilBeBri opened 3 years ago

emilBeBri commented 3 years ago

So I've been trying to use this function for a while, but when using it and writing the file back to disk with df2bib(), Jabref suddenly told me

scrot_2021-03-03_20-54-10

I found this puzzling, and tried troubleshooting the error. While doing this, I then stumbled upon something where using the eprint field, a entry that looked like this (very minimal)

@inproceedings{joulin2017bag,
    title = "Bag of Tricks for Efficient Text Classification",
    author = "Joulin, Armand",
    year = "2017",
    eprint={1607.01759},
}

was transformed into this

@Inproceedings{joulin2017bag,
  Author = {Joulin, Armand},
  Title = {Bag of Tricks for Efficient Text Classification},
  Year = {2017},
  Eprint..1607.01759.. = {1607.01759}
}

This problem does not appear to related to issue with the empty text fields that I was actually investigating. This seems to be an issue related to the package only having a specific set of fields, and any not in that list will be parsed with the name of the values IN the field as part of the column names. This means that Eprint gets converted into Eprint..1607.01759..

So there are two serious bugs here, and only one of them is immediatly reproducible. I give up for now :)

I conclude that this package should be used with caution, and you should definitely NOT write back into a bibtex file that you need for anything reliable.

GeoBosh commented 3 years ago

Please note that your input bibtex entry is invalid - for entries of type inproceedings the field booktitle is compulsory for the standard bibtex styles. Also, I am surprised that bibtex even parsed it, since my slightly older version of package bibtex, v0.4.2.2, gives this:

> bibtex::read.bib("bibtex_issue.bib")
ignoring entry 'joulin2017bag' (line 1) because :
    A bibentry of bibtype ‘InProceedings’ has to specify the field: booktitle

Please note that the tone of your posting is insulting, maybe unintentionally. You have given a good description of your problem, perceived or not, for the maintainers (I am not among them) to look at. And that should have been it.

emilBeBri commented 3 years ago

I don't agree with you that it's insultating - to you, perhaps, but not generally. Of course that's not my intention: I'm stating that this bug seems to be so severe, that it should not be used to write a .bib-file, since this bug will in all likehood scramble the original bib-input. This seems to me to be aa very important point to get through, before anybody wrecks something important.

mwmclean commented 3 years ago

The ALL CAPS and double exclamation points in the title... so authoritative and you couldn't even bother to create a reproducible bug report. Your post is 'insultating' to any developer. I conclude your GitHub comments "shouldn't be used" EXCLAMATION MARKS!!!!!!

emilBeBri commented 3 years ago

This bug nearly ruined an important .bib file i was using, so the all caps and !! was with the intention of making sure nobody else ruined something important they were doing.

I find your suggestion that I "didn't bother including a reprex" to be rude - I spent quite a bit of time figuring out what the error was, so I could post it here.

But of course, if two different people both thinks I wrote the report in a rude manner, I will take that into consideration. There was no bad faith involved, however. The all caps and exclamation mark was probably a bad idea.

dieghernan commented 2 years ago

Leaving aside other discussions, I think there is a fair point on here. I would propose to implement a mechanism for creating a backup of the bib file just in case something goes wrong. I already did this on cffr:

https://github.com/ropensci/cffr/blob/855608c8f6d3d7da0180db3e978143a288b6558b/R/write_bib.R#L65:L79


  # If exists creates a backup
  if (file.exists(file)) {
    for (i in seq(1, 100)) {
      f <- paste0(file, ".bk", i)
      if (!file.exists(f)) break
    }

    if (verbose) {
      message(
        "Creating a backup of ",
        file, " in ", f
      )
    }
    file.copy(file, f)
  }