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

Official stance on getproperty, setproperty! and propertynames and docs #27120

Closed
mauro3 opened this issue May 16, 2018 · 4 comments · Fixed by #27151
Closed

Official stance on getproperty, setproperty! and propertynames and docs #27120

mauro3 opened this issue May 16, 2018 · 4 comments · Fixed by #27151
Assignees
Labels
docs This change adds or pertains to documentation needs docs Documentation for this change is required

Comments

@mauro3
Copy link
Contributor

mauro3 commented May 16, 2018

It is a bit unclear to me whether those are now the standard way to access type-fields or not? Presumably yes, but then why are they not exported and why are getfield, setfield! and fieldnames still exported if they should not be used? A clear stance on the "official-ness" of those would be good. Maybe pre-0.7 would be good if exports are to be added/removed.

Once that is cleared-up, the docs need updating:
They are missing adequate doc-strings and are almost fully absent from the manual. To cite @stevengj: "We really need more documentation on how to use this, explaining the arguments to getproperty, how to fall back to getfield, implications for performance and type stability, best practices...."
#24960 (comment)

Also, other places, such as https://docs.julialang.org/en/latest/manual/types/#Composite-Types-1 need updating if "properties" are "official".

@fredrikekre fredrikekre added the docs This change adds or pertains to documentation label May 16, 2018
@JeffBezanson
Copy link
Member

Yes, let's just export them.

@stevengj stevengj modified the milestone: 1.0 May 16, 2018
@stevengj stevengj added the triage This should be discussed on a triage call label May 16, 2018
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label May 17, 2018
@mbauman
Copy link
Member

mbauman commented May 17, 2018

Triage is in agreement with Jeff here — they just need to be exported and documented.

@stevengj stevengj added this to the 1.0 milestone May 17, 2018
@stevengj
Copy link
Member

Added to the milestone since the export needs to be added before 0.7 release.

@mauro3
Copy link
Contributor Author

mauro3 commented May 20, 2018

Maybe this could be kept open for the doc-part? But remove the 1.0 milestone.

@StefanKarpinski StefanKarpinski added the needs docs Documentation for this change is required label May 20, 2018
@StefanKarpinski StefanKarpinski modified the milestones: 1.0, 1.0.x May 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation needs docs Documentation for this change is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants