Skip to content

Conversation

@megies
Copy link

@megies megies commented Nov 14, 2019

WIP - DO NOT MERGE

only appropriate for NS-EW aligned raster but I guess thats true for the
whole code base
Copy link
Owner

@ozak ozak left a comment

Choose a reason for hiding this comment

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

What is the benefit of this? Just wondering? Less memory?

Copy link
Owner

@ozak ozak left a comment

Choose a reason for hiding this comment

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

Not sure which duplication you refer to. The function is defined only once.

Comment on lines 291 to 292
self.geot = geot
self.nodata_value = nodata_value
Copy link
Owner

Choose a reason for hiding this comment

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

Would there be a reason to not use the @property for the other properties?

Copy link
Author

Choose a reason for hiding this comment

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

There's no need, unless you want to add e.g. type checks when these get set.
Main reason to make those other attributes into properties is, they are derived values and should not be stored by itself but rather looked up from their "parents".

@ozak
Copy link
Owner

ozak commented Nov 20, 2019

Thanks for taking the time! If you are able to finish these let me know and I will merge the PR soon.

@megies
Copy link
Author

megies commented Nov 25, 2019

What is the benefit of this? Just wondering? Less memory?

Somehow this comment is not linked to code, so not sure what change you were commenting on..

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