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

spool add global config for configuration of hardcoded spool quadtree params #94

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hvasbath
Copy link
Member

@hvasbath hvasbath commented Dec 8, 2022

No description provided.

@hvasbath
Copy link
Member Author

hvasbath commented Dec 8, 2022

Any comments @miili regarding the concept of that? I just rebased. I experience it now fairly often-also with others that it is necessary to have finer Quadtrees...

Copy link
Member

@miili miili left a comment

Choose a reason for hiding this comment

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

Thanks for rebasing this PR. Can you please remove all the dead code. I think we can get away with a very simplistic config.

Comment on lines +55 to +58
class ConfigBase(Object):
@classmethod
def default(cls):
return cls()
Copy link
Member

Choose a reason for hiding this comment

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

Not needed imo. Implement in KiteConfig

Comment on lines +84 to +85
x = op.expanduser(op.expandvars(x))
return x
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
x = op.expanduser(op.expandvars(x))
return x
return op.expanduser(op.expandvars(x))


kite_dir_tmpl = os.environ.get(
'KITE_DIR',
os.path.join('~', '.kite'))
Copy link
Member

Choose a reason for hiding this comment

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

Please use pathlib.Path throughout.

Comment on lines +32 to +34
class PathWithPlaceholders(String):
'''Path, possibly containing placeholders.'''
pass
Copy link
Member

Choose a reason for hiding this comment

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

Never used.

Comment on lines +37 to +52
class VisibleLengthSetting(Object):
class __T(TBase):
def regularize_extra(self, val):
if isinstance(val, list):
return self.cls(key=val[0], value=val[1])

return val

def to_save(self, val):
return (val.key, val.value)

def to_save_xml(self, val):
raise NotImplementedError()

key = String.T()
value = Float.T()
Copy link
Member

Choose a reason for hiding this comment

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

Never used.

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.

2 participants