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

Update Styles to 2015 or even 2021 BJCP Guidelines #125

Closed
rocketman768 opened this issue Dec 13, 2015 · 29 comments
Closed

Update Styles to 2015 or even 2021 BJCP Guidelines #125

rocketman768 opened this issue Dec 13, 2015 · 29 comments

Comments

@rocketman768
Copy link
Member

@mikfire
Copy link
Contributor

mikfire commented Jun 3, 2016

This really needs to be done. Ugh. Guess I will play with it this weekend.

@dpettersson
Copy link
Contributor

How did it go with this?
Is it still open or has it beeen addressed?

@mikfire
Copy link
Contributor

mikfire commented Nov 10, 2016

Still open, really. The BJCP has changed in the decade since beerXML was designed, and I couldn't find any reasonable mapping between the two. This sent me on one of my (unfortunately common) tirades about how horrible beerXML is.

I then sat down and designed a JSON schema (for validation) for both BJCP and the Brewer's Association styles, hoping to leverage the work already done by brewerwall. I never could quite figure out how to combine the two schema definitions though.

And there it sits, because every time I think about picking it up again, I start to get a headache.

@dpettersson
Copy link
Contributor

Is there any possibility to see the work you have done so far?

Den 10 nov. 2016 5:18 fm skrev "mikfire" [email protected]:

Still open, really. The BJCP has changed in the decade since beerXML was
designed, and I couldn't find any reasonable mapping between the two. This
sent me on one of my (unfortunately common) tirades about how horrible
beerXML is.

I then sat down and designed a JSON schema (for validation) for both BJCP
and the Brewer's Association styles, hoping to leverage the work already
done by brewerall. I never could quite figure out how to combine the two
schema definitions though.

And there it sits, because every time I think about picking it up again, I
start to get a headache.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#125 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ALLp4iLBxiNCHm1QD9NrJh0IrDDce_7Fks5q8psNgaJpZM4G0X5I
.

@mikfire
Copy link
Contributor

mikfire commented Nov 10, 2016

https://github.com/mikfire/brewtarget/tree/feature/json

Because I lack imagination, the files are in the json directory. If you want the input XML for the BJCP guide, you will need to get it from meanphil (https://github.com/meanphil/bjcp-guidelines-2015). I included the scripts I used to generate the JSON from XML (xmltojson.pl), as well as the script I was using to test the validation (validate.pl).

@kapinga
Copy link
Contributor

kapinga commented Mar 6, 2017

I'm happy to work on this if it'll be helpful (no promises on actually doing anything useful, since I just started trying to develop BrewTarget a week or two ago).

@mikfire, can you explain briefly explain what you're hoping to do with the JSON style definitions, once you have them? AFAICT, you're hoping to create on or more JSON files that are compatible with a modified version of add-to-default-db.py so that we can programmatically convert the BJCP/BA style guides into entries that can be added to the default database. Is that correct?

From poking around the style table in default_db.sqlite, it seems like we're going to need to make a lot of changes to fully reflect the new style guides. The current schema doesn't include a revision field, and I think only includes BJCP 2008 styles. There's several other missing fields that are included in your JSON schema (e.g. aroma, appearance, flavor, mouthfeel, impression, etc.) that are not part of the existing database schema.

The other issue I see is that the style numbers have changed significantly, so we need to completely overhaul the list of styles used. I imagine we'd go about it something like the following:

  1. Add new columns to the style table to include appropriate information from the new style guidelines.
  2. Fill in required information (e.g. revision) for the existing style entries in the default db. This change would need to be propagated into users' existing databases (i.e. I think this has to be another DB version upgrade).
  3. Add the new style guidelines to the default DB, using the add-to-default-db.py script as new style entries.
  4. Update the BrewTarget UI to surface the new columns as appropriate. This should include making the style guide and revision obvious when the user is selecting a style (e.g. '1A: American Light Lager (BJCP 2015)')
  5. The new styles should be forced to be at the top of the style list. (I don't want to have to scroll past a bunch of out-of-date styles every time I'm designing a new recipe.) Alternatively, we have have a user option to only show styles from one guideline revision. Styles from other revisions would be hidden by setting display to 0 in the database.
  6. Figure out how to import/export to BeerXML with as little fidelity loss as possible. BeerXML is explicit that extra undefined tags can be included in files, and parsing software should ignore those ones it doesn't understand. At a first pass we can just add in the new fields when exporting. (It might be worthwhile to figure out how other software has worked around this to keep our solution as cross-compatible as possible.) For imports, if our non-standard fields aren't there, we should make a best guess on the revision based on the style guide name, category name & number, and the style name & number. Anything that matches a known combination of names and numbers (BJCP 2015 1C would be 'Cream Ale', vs BJCP 2008 1C's 'Premium American Lager') should have any information not in the BeerXML pulled from the appropriate style in the user's database.

That was way more long-winded than I expected. Do these changes make sense, or am I over-thinking this somehow?

@mikfire
Copy link
Contributor

mikfire commented Mar 6, 2017

The problems you mentioned at the beginning are why I gave up. I couldn't find any reasonable mapping from the current guides into the festering sore that is BeerXML.

My goal was to use this as a launch pad to killing BeerXML and replacing it with something more in line with modern data exchange, something that could be validated, something that allowed us to easily consume external resources (like brewerwall and the BJCP XML files) so we didn't have to maintain this by hand, etc.

In the order you put them forward, here are some ideaas:

  1. New columns? Maybe a new table? This is a pretty significant change, and I am not sure we can graft the new information onto the old. Of course, this raises a whole slew of new problems for which I don't have good ideas.
  2. Can easily be done -- that is what the DatabaseSchemaHelper class does.
  3. I would prefer something ... more automatic. Like a webservices call to brewerwall and something equally clever to get the BJCP XML guidelines. It also doesn't help that I cannot read or code python :)
  4. Yup
  5. This part is actually easy. We would just mark the old versions as either deleted or not displayed. The recipes that used the old versions would still show their local copy, but it wouldn't show in either the tree or the drop down lists.
  6. That is going to be really, really hard. Or really, really easy if we just invent rules like "To get this BeerXML attribute, we will simply concatenate these three fields from the BJCP 2015 spec or these two fields from the BA spec" and don't worry too much about coherence. It is another reason I think the BeerXML specification needs to die a very grim and horrible death.

@kapinga
Copy link
Contributor

kapinga commented Mar 7, 2017

If modifying the database is hard (and you know far better than I on this) it seems that we're stuck with the fact that BrewTarget's internal representation is based on BeerXML. I like your idea of pulling data from various external sources automatically, but we're still stuck with the fields we have within the working database.

The easiest short-term solution would be to shoe-horn the more detailed descriptive features (aroma, flavor, mouthfeel, etc.) from BJCP 2015 into the Notes field in BeerXML and the BrewTarget style database. I'm thinking of using YAML for this so that all the fields are still user-readable if they get exported to other software. For now, it shouldn't be hard for me to write a Python script (or modify your Perl one) to format things into the appropriate format, and in the future this could be fully automated.

The more complex change of automatically importing new styles and ingredients sounds really cool, but IMO isn't worth holding up the BJCP 2015 style guidelines at this point.

@bdleedy
Copy link

bdleedy commented Mar 7, 2018

Can't this be done manually? It's seemingly taking 2+ years to update this AND the BJCP hasn't been updated in that time so creating a script to do this once seems like a bit of a waste. Is it just cracking the SQLite and entering the data (and adding new fields/columns)? It would also be nice to either change category_number to something that sorts better or to add a leading zero. Give me direction and I'll go with it.

@bdleedy
Copy link

bdleedy commented Mar 8, 2018

OK, so after looking at this, I have a better understanding of most things.

  1. Definitely need new columns. I would even say go for broke and new use two new tables, style_cat to hold the main category and data and style_subcat with FK to style_cat. It LOOKS like there was an odd attempt at this with style_children? This route definitely makes an upgrade more challenging but seemingly doable. I'm envisioning either renaming the current db file or use of some sort of temp file.
  2. I'm not familiar with the Brewerwall guidelines. Grabbing those would be easy but how often are they really updated? Same with BJCP. They seem pretty static and I agree that this should not hold up and update.
  3. Are you saying that if you are importing a BeerXML that you want to have it match to the correct style in the current db? I would say that the import checks for category_number and style_letter and if more than one, it prompts for a decision?

@bdleedy
Copy link

bdleedy commented Mar 8, 2018

I manually created/modified tables. Created style_category, deleted style_children, renamed style to style_subcategory. I then wrote a C# .NET to rip the XML and dump it into the new fields. Worked perfect.

SO. Now comes the automation of all of that. Grab old data, create/modify tables, return old data, import new data. Apparently in Python? I'd rather not learn Python but I am willing to do all of the leg work with writing the SQLite comands if someone else will do the Python.

If anyone is interested in looking at my file or the CREATE commands. I can get them to them.

@pricelessbrewing
Copy link
Contributor

Not sure if it's in a useable format for brewtarget, but https://github.com/beerjson/beerjson/blob/master/tests/styles/bjcp_styleguide-2015.json might be of assistance.

@bdleedy
Copy link

bdleedy commented Mar 9, 2018

I took one look at it, YES. Formatted much better.

Changing the database is turning into quite the sprawl. Edit here, edit there, create new here.....

@pricelessbrewing
Copy link
Contributor

pricelessbrewing commented Aug 22, 2018

Aside from the database mapping or manual entry if it comes to it. @bdleedy Are you still working on this? I

What I think would be great is to have style groups for the different organizations (BJCP 2008, bjcp 2015, GABF, Brewers association, etc). Then the style editor window should be outlined similar to the ingredients, where the style letter and category number are easily visible and sortable, and can be quicksearched by typing "american" or "pale" etc.

This will give us more flexibility with changing styles, and increase workflow and usability. The window on the left (no idea what this is called in brewtarget) was updated during the 2.3->2.4 development but the style editor was not.

@bdleedy
Copy link

bdleedy commented Aug 22, 2018 via email

@pricelessbrewing
Copy link
Contributor

I'm going to try and take this on. No promises, but I think I've figured out how to map the files from https://github.com/beerjson/beerjson/tree/master/tests/styles back into beerxml, at which point I can either pass it off to someone to mass add them to the brewtarget db, or else I can import them one by one if necessary.

@pricelessbrewing pricelessbrewing self-assigned this Oct 1, 2018
@pricelessbrewing
Copy link
Contributor

https://github.com/couchguy/BJCP-Style-Guide-to-BeerXML/blob/master/2018styles.xml

Found this, appears to be formatted correctly! @dpettersson Should I just add these styles to my db manually then submit a PR with an updated db or is there a quicker way?

@Wyrmwood
Copy link

Thanks for the link! Updated mine manually by truncating styles, then loading that file (sorted with python and umlauts removed). It exits the app when you import it, but it appears to work fine after doing a restore.

@pricelessbrewing
Copy link
Contributor

@Wyrmwood can you submit a PR with the updated styles then? I never got around to doing it manually.

@Wyrmwood
Copy link

Well, I removed the 2008 styles from mine...

@pricelessbrewing
Copy link
Contributor

Gotcha. Well could you make a fork, or upload your db somewhere like Dropbox or something then I can just merge the two db.

@Wyrmwood
Copy link

OK. Noticed some issues when I exported, so started over. Here's couchguy's, sorted and converted to utf-8, then imported to the default database, so it shows up after the 2008. Please Note: his is missing the specialty IPAs. After I add those back to mine, I'll post the xml file.
database.zip

@Wyrmwood
Copy link

Specialty_IPAs.zip

@Wyrmwood
Copy link

Wyrmwood commented Jan 18, 2019

2015_styles_ext.zip
These are the updated styles (with specialty IPAs), also in utf-8, and sorted. The sorting would not be necessary if the dropdown for styles sorted them. The styles view is sorted and when you add a new style, it is sorted in that view, but will be in db order in the dropdown. Also, all the umlauts and other unusual characters are escaped in xml and render correctly in Brewtarget.

@xzfantom
Copy link
Contributor

I have a suggestion about styles and some other resources. Maybe better add some function to app for loading such xml (or json) by user. So if somebody want to use bjcp 2008 or any other style library (or translated in native language) it can do it. We can ship resource xml (or json) file with program and load at first start or give user opportunity to do it by himself.

@matty0ung
Copy link
Contributor

@xzfantom The good news is that this functionality already exists. I'm doing some work to improve XML importing but I think even the current version of the software can import styles from BeerXML. If you do "File > Import Recipe", it can actually import not just recipes but any other BeerXML document, including styles, hops, etc.

I am going to change the name of the menu item to "Import from File" to make this clearer. And my new version of the import will be a bit more robust (ie less likely to crash!) and more helpful about error messages and telling you what it has read in, but I'm not changing the base functionality.

@xzfantom
Copy link
Contributor

@matty0ung that's cool. I just started to learn BT and it's code so this part was not obvious)

@matty0ung
Copy link
Contributor

@xzfantom FYI, I took the "2015_styles_ext.zip" file uploaded by @Wyrmwood, extracted "2015_styles_ext.xml" and, with one small edit, was able to import all 142 style records into Brewtarget patched with #515. (The patch tells you how many records you imported, and will skip over duplicates if you try to import the same file twice by the way.)

The edit was to insert the following line at the beginning of the file:
<?xml version="1.0" encoding="ISO-8859-1"?>
AIUI, strictly a file is not valid BeerXML without this initial line. (If I am wrong or we want to be more accepting of files without this line, then it would be small work to make it optional. I will take a view from @mikfire on this.)

@matty0ung
Copy link
Contributor

I have added the BJCP 2021 styles to our default content as part of #799.

@matty0ung matty0ung changed the title Update Styles to 2015 BJCP Guidelines Update Styles to 2015 or even 2021 BJCP Guidelines Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants