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

ParticleSlice lacks support for particle properties #958

Closed
RudolfWeeber opened this issue Nov 28, 2016 · 10 comments
Closed

ParticleSlice lacks support for particle properties #958

RudolfWeeber opened this issue Nov 28, 2016 · 10 comments
Milestone

Comments

@RudolfWeeber
Copy link
Contributor

This should be resolved by a generic implementation of setattr and getattr in the ParticleSlice class
If ParticleSlice does not have an implementation for the given property,
The getter and seetter should call the property with the given attribute name of the ParticleHandle class for all particles in the slice.
The getter additionally has to determine the number of values for the particle property by obtaining the result for the first particle in the slice and then creating the approprate numpy array.

Those properties of ParticleSlice already there, which only call properties of ParticleHandle should be removed, because it's lots of copied code.
The properties of ParticleSlice directly calling core methods should probably stay, because they might offer better performance.

@RudolfWeeber RudolfWeeber added this to the Espresso 4.0 milestone Nov 28, 2016
@RudolfWeeber
Copy link
Contributor Author

I was going to implement this. However, as we now moved to sphix documentation, there is a catch.

If this is implemented as suggested above, the individual properties of
ParticleSlice
such as
ParticleSlice.pos
do not have docstrings anymore.
We can either

  1. decide to accept this drawback, and put a remark into the class docstring, that all properties of ParticleHandle are available. This would remove some 300 lines of code from particle_data.pyx and automatically support all properties of ParticleHandle.

  2. or declare all properties of ParticleHandle explicitly in ParticleSlice like
    property pos:
    """docstring"""
    def get(self):
    self.generic_getter("pos")
    def set(self,value):
    self.generic_setter("pos",value)

This would allow for docstrings, but there would still be a lot of nearly identical code. It would also not be automatically synced with the properties of ParticleHandle

My personal preference is 1.
Opinions?

@fweik
Copy link
Contributor

fweik commented Apr 26, 2017

I'm in favor of 1

@mkuron
Copy link
Member

mkuron commented Apr 26, 2017

Can docstrings be set programmatically? I seem to recall that @fweik ran into an issue there, but since this is Python, there must be a way to do it.

@fweik
Copy link
Contributor

fweik commented Apr 26, 2017

No, they can not. The reason is that they are typically read on module load.

@mkuron
Copy link
Member

mkuron commented Apr 27, 2017

Are you sure? A (simplified) test works:

docstr.py:

def a():
	"""Does nothing"""
	pass

def b():
	a.__doc__ = "Still does nothing"

b()

test_docstr.py:

import docstr

if __name__ == "__main__":
	 help(docstr.a)      # =>     Still does nothing

@hmenke
Copy link
Member

hmenke commented Apr 27, 2017

@mkuron Does this also work in Sphinx?

@mkuron
Copy link
Member

mkuron commented Apr 27, 2017

Doesn't Sphinx just import all modules and use introspection to read the docstrings?

I'll test it if you tell me how I can run Sphinx on a single Python file.

@RudolfWeeber
Copy link
Contributor Author

One can probably popoulate the docstrings at module load.
However, I did not manage to add properties to a class programatically, the way methods can be added:

class C:
pass

def gen_method(name):
def method(self):
return name
method.doc ="Docstring for "+name
return method

for mname in "a","b","c":
method=gen_method(mname)
setattr(C,mname,method)

c=C()
print c.a(),c.b(),c.c()
print help(c.b)

@mkuron
Copy link
Member

mkuron commented Apr 27, 2017

Your indentation got broken by GitHub, I've cleaned it up:

class C:
	pass

def gen_method(name):
	def method(self):
		return name
	method.__doc__ = "Docstring for " + name
	return method

for mname in "a","b","c":
	method = gen_method(mname)
	setattr(C,mname,method)

c=C()
print c.a(),c.b(),c.c()
help(c.b)

Actually, since properties are just special kinds of methods, you can add them just as easily:

class C:
	@property
	def name(self):
		"""returns Hello"""
		return "Hello"

c = C()
print c.name

@property
def new_value(self):
	"""returns World"""
	return "World"
C.value = new_value
print c.value       # =>   World

help(c)             # =>   [...] value [...] returns World [...]

@RudolfWeeber
Copy link
Contributor Author

RudolfWeeber commented Apr 27, 2017 via email

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

No branches or pull requests

4 participants