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

Reformated to be a proper package #10

Closed
wants to merge 4 commits into from

Conversation

rodrigob
Copy link

@rodrigob rodrigob commented Mar 8, 2016

Here is my pull request to address issue #9.
I squashed all my mini-changes into a big commit (other repositories like to do that).

I tried a fresh install into a virtualenv and it seemed to work fine. So unless I messed-up with my git manipulations, this should solve the issue.

Note that setup.py will also install pydensecrf.util as required.

I created my own playground applications to use it. Maybe util_inference_example.py should be moved of the package, and have it call import pydensecrf.densecrf and import pydensecrf.utils; so as to show the proper use of the library.

@lucasb-eyer
Copy link
Owner

Thanks a lot, just tested and it seems to work for me too! Good call on squashing, I prefer that too.

Few minor comments:

  • The history of SparseAssign.h is weird, if it's easy to fix and squash+force-push that, I'd be grateful; but nevermind if it's a hassle.
  • *~ and .idea don't belong into a project's .gitignore but rather into your global .gitignore, so would you mind taking them out, squashing and force-pushing?
  • I agree with your comment on util_inference_example.py. Could you do that? (and import pydensecrf.densecrf as dcrf so we set a nice example.) I don't mind whether you squash this one or keep this commit separate.

@lucasb-eyer
Copy link
Owner

Oh, and if you write something like "Fixes #9" into the commit message, github will automatically close that issue when merging this PR.

@rodrigob
Copy link
Author

rodrigob commented Mar 8, 2016

Just updated my branch with these changes. I had problems with github and squashed commits (I am not a git guru, more of a hg guy), so I ended having "repeated commits".
I hope you can fix the commits history on your side.

@rodrigob
Copy link
Author

rodrigob commented Mar 8, 2016

Just noticed a minor change lost in my git fights. Pushed back as one extra commit.

@lucasb-eyer
Copy link
Owner

Merged manually with clean history, See c0550af, 19119d3, and 1463feb. If you want to take credit, I recommend you associate the e-mail address that you committed with (@mpi...) with your github account!

@lucasb-eyer
Copy link
Owner

Also, closing manually since it looks like github doesn't auto-detect manual merges.

@lucasb-eyer lucasb-eyer closed this Mar 8, 2016
@lucasb-eyer
Copy link
Owner

Thanks a lot for sharing this with us!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants