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

Override getproperty, setproperty! and propertynames for AbstractDataFrame and DataFrameRow #1406

Merged
merged 1 commit into from
May 31, 2018

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Apr 28, 2018

Provides a nice shorthand syntax for simple getindex and setindex! uses, with autocompletion,
and makes DataFrameRow more like NamedTuple.
This requires replacing all direct field accesses with calls to accessor functions.
Do not mention this feature in the manual yet since it is not available on a released Julia
version. Most uses of df[:col] will better be changed to df.col when Julia 0.7 is released.

(CI failure on 0.7 is fixed with DataStreams master.)

Fixes #1336.

…Frame and DataFrameRow

Provides a nice shorthand syntax for simple getindex and setindex! uses, with autocompletion,
and makes DataFrameRow more like NamedTuple.
This requires replacing all direct field accesses with calls to accessor functions.
Do not mention this feature in the manual yet since it is not available on a released Julia
version. Most uses of df[:col] will better be changed to df.col when Julia 0.7 is released.
Base.getproperty(df::AbstractDataFrame, col_ind::Symbol) = getindex(df, col_ind)
Base.setproperty!(df::AbstractDataFrame, col_ind::Symbol, x) = setindex!(df, x, col_ind)
# Private fields are never exposed since they can conflict with column names
Base.propertynames(df::AbstractDataFrame, private::Bool=false) = names(df)
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what's the best solution here. This situation is going to arise for other types where field names are dynamic. We could also throw an error when private == true.

@ararslan
Copy link
Member

Fixes #1336?

Copy link
Member

@bkamins bkamins left a comment

Choose a reason for hiding this comment

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

@nalimilan Very nice - thank you. I have reviewed the code and it looks OK.

I have one question though (unfortunately I am unable to test it locally currently).

How does the . notation work with non-standard column names. What I mean by this is a name like Symbol("Column 1"). It is accessible via the method call I guess - right? But can it be accessed via . somehow or it is not possible? I guess those things should be eventually mentioned in the docs.

@nalimilan nalimilan merged commit fb94cd6 into master May 31, 2018
@nalimilan nalimilan deleted the nl/property branch May 31, 2018 16:37
@nalimilan
Copy link
Member Author

Thanks, I had forgotten about this!

How does the . notation work with non-standard column names. What I mean by this is a name like Symbol("Column 1"). It is accessible via the method call I guess - right? But can it be accessed via . somehow or it is not possible? I guess those things should be eventually mentioned in the docs.

I'm not sure, but indeed I think you'd need to call getproperty. These clearly need docs, but until 0.7/1.0 are released it would be misleading to use them in the manual.

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.

Add column lookup with Base.getproperty?
3 participants