Skip to content

BLD: use tempita for hashtable #13815

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

Merged
merged 1 commit into from
Jul 28, 2016
Merged

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Jul 27, 2016

@sinhrks sinhrks added Build Library building on various platforms Clean labels Jul 27, 2016
@sinhrks sinhrks added this to the 0.19.0 milestone Jul 27, 2016
@codecov-io
Copy link

codecov-io commented Jul 27, 2016

Current coverage is 85.23% (diff: 100%)

Merging #13815 into master will not change coverage

@@             master     #13815   diff @@
==========================================
  Files           140        140          
  Lines         50420      50420          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          42975      42975          
  Misses         7445       7445          
  Partials          0          0          

Powered by Codecov. Last update cc216ad...912aff8

if val == val:
{{endif}}

{{tab}}k = kh_get_{{dtype}}(self.table, val)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{tab}} is pretty ugly! If tempita doesn't have a way to handle conditional indentation I would consider simply making separate blocks for each condition. That will be slightly more code but easier to understand.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, using if-else is clearer in this case. fixed.

@jreback
Copy link
Contributor

jreback commented Jul 27, 2016

be sure to add to the linter as well.

@sinhrks
Copy link
Member Author

sinhrks commented Jul 27, 2016

@jreback OK. linting is enabled for all src/*.pxi.in by default.

@jreback jreback merged commit 12c8ce6 into pandas-dev:master Jul 28, 2016
@jreback
Copy link
Contributor

jreback commented Jul 28, 2016

thanks @sinhrks I know you wanted to add other types (e.g. int32), so can make another issue for that, but this is great.

@sinhrks sinhrks deleted the tempita_htable branch July 28, 2016 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms Clean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLN: template hashtable.pyx
4 participants