funginstitute / patentprocessor

BSD 2-Clause "Simplified" License
68 stars 31 forks source link

Fix application ID parsing, etc. #80

Closed billyeh closed 10 years ago

billyeh commented 10 years ago
billyeh commented 10 years ago

By the way, integration tests have gone a little crazy since the last changes with this celery thing, and I'm looking into it now because I also wanted to add more for all 13 years of application schemas just to be safe. I'll keep the repo updated with new pull requests/issues when I find them.

gtfierro commented 10 years ago

I removed the external celery stuff, so now it all runs in a single thread. I'll hack on it today and let you know if anything needs to be changed.

billyeh commented 10 years ago

It also seems like we'll need to change application ID parsing for grants and/or applications, since they have the same information, just in slightly different formats, right now. I can change the applications' stuff because it seems like having [year]/id is redundant, and grants just have the id.

gtfierro commented 10 years ago

Can you make the necessary changes in the grant/application parsers so that we get the IDs and numbers as they appear in usptofixed.application and application.application? We need to change usptofixed.application.id, application.application.number and application.application.id

billyeh commented 10 years ago

Yup - I'll add to this pull request later this week.

gtfierro commented 10 years ago

I promise I'll look into merging this PR this week, Bill

billyeh commented 10 years ago

Sure, whenever you have time. Don't forget that in this pull request, the application id is still in the form "year/8-digit serial number", and the number is in the form "8-digit serial number".

gtfierro commented 10 years ago

Isn't that what we decided on using?

gtfierro commented 10 years ago

As in, that's what it's like in the current usptofixed/apptest database?

billyeh commented 10 years ago

Yup, exactly. Sorry, just wanted to clarify.

billyeh commented 10 years ago

I don't even know if this helps, but I've rebased so that if there's merge conflicts you don't have to fix them for the pull request. There was also a typo on a print and we might want to add a dependency for joblib.

gtfierro commented 10 years ago

Thanks for the rebase, Bill. I have a few changes in my own branch that I want to push in order to finish the contract. I'll be doing that sometime soon, and then I'll ping you so you can do a final rebase and then we'll merge in the PR. Sound good?

billyeh commented 10 years ago

Of course. Thanks for the reply! When things are stable I'll be able to add tests too :)

billyeh commented 10 years ago

Thanks for merging. Will close! :+1: