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

code reorganization (preparation for Arrow implementation) #73

Closed
wants to merge 5 commits into from

Conversation

ExpandingMan
Copy link
Collaborator

@ExpandingMan ExpandingMan commented Jan 11, 2018

I have reorganized the source code in preparation of rewriting Feather based on an underlying Arrow implementation.

Substantive Changes

Formatting Changes

  • I have split the file "Feather.jl" into several distinct files. It is intended that these will completely change again once the Arrow implementation is complete. There is also a subdirectory called "arrow'. This is where Arrow development will occur for the time being until it is appropriate to split it off into a separate package.
  • I have added several short convenience functions for commonly occurring operations.
  • Various lines of code have been split into multiple lines for readability. I personally find very long run-on lines very difficult to parse. Also, the code is now easier for people who use editors with multiple columns to read.
  • I added a tiny amount of (non user-facing) documentation.

Let me know if this seems acceptable.

@ExpandingMan
Copy link
Collaborator Author

Looks like tests are getting another method ambiguity error. Will look into this tomorrow.

@ExpandingMan
Copy link
Collaborator Author

ExpandingMan commented Jan 12, 2018

I had accidentally left out some type assertions. Should be fine now.

Looks good on 0.6 on Linux. Mac build is hanging for some reason.

@ExpandingMan
Copy link
Collaborator Author

This may be sort of a moot point as Feather will pretty much have to be completely re-written with Arrow fully implemented. See #72 and my progress here.

@ExpandingMan
Copy link
Collaborator Author

This is no longer relevant. See my PR #78.

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.

1 participant