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

docs question #369

Closed
spaette opened this issue Nov 10, 2023 · 7 comments · Fixed by #370
Closed

docs question #369

spaette opened this issue Nov 10, 2023 · 7 comments · Fixed by #370

Comments

@spaette
Copy link
Contributor

spaette commented Nov 10, 2023

Does the first paragraph need to be rewritten?

I intend to submit a pull request pending clarification.

$ ed -s DataFramesMeta.jl/src/macros.jl <<<'2798, 2820p'
### Details

Both the left- and right-hand side of an expression specifying a column name assignment
can be either a `Symbol` or an `AbstractString` (which may contain apces) escaped with `
$DOLLAR`. For example `:new = ...`, and `$(DOLLAR)"new" = ...` are both valid ways
of assigning a new column name.

This idea can be extended to pass arbitrary right-hand side expressions. For example,
the following are equivalent:

```
@rename(df, :new = :old1)
```

and

```
@rename(df, :new = $("old_col" * "1"))
```

The right-hand side can additionally be an `Integer`, escaped with $(DOLLAR), to indicate
column position. For example, to rename the 4th column in a data frame to a new name, write
`@rename df :newname = $(DOLLAR)`.
$ 
@pdeffebach
Copy link
Collaborator

How would you re-write? What do you think is wrong (aside from the obvious typo)

@spaette
Copy link
Contributor Author

spaette commented Nov 11, 2023

Perhaps I misunderstand the author's intent in that paragraph.

obvious typo

Were apces the typo would the fix possibly be either of ellipses or an ellipsis?

@spaette
Copy link
Contributor Author

spaette commented Nov 11, 2023

Aside from the correction of that paragraph the repo had a second typo.

$ grep -nr dont DataFramesMeta.jl
DataFramesMeta.jl/test/parsing.jl:125:    # Some errors when we dont have keyword arguments, since
$ 

@pdeffebach
Copy link
Collaborator

Sure. Please open a PR with these fixes.

@spaette
Copy link
Contributor Author

spaette commented Nov 13, 2023

Were apces the typo would the fix possibly be either of ellipses or an ellipsis?

@bkamins

Would you chip in on this ticket?

Looking at the string apces it occured to me the author's intent may have been spaces.

Otherwise the singular an ellipsis would seem more fitting than the plural ellipses.

@pdeffebach
Copy link
Collaborator

No need to ping Bogumil. You are entirely correct. Please open a PR with those changes.

@spaette
Copy link
Contributor Author

spaette commented Nov 13, 2023

entirely correct

Evidently not in regards to my interpretation of what you wrote in your comments.

Pull has been fixed with a soft reset performed.

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 a pull request may close this issue.

2 participants