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

boilerplate for inasafe report processing algorithm #5070

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

myarjunar
Copy link
Collaborator

Copy link
Contributor

@Gustry Gustry left a comment

Choose a reason for hiding this comment

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

Nice to see InaSAFE moving to Processing

@@ -0,0 +1,2 @@
# coding=utf-8
Copy link
Contributor

Choose a reason for hiding this comment

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

We are now in Python 3, not needed anymore

import tempfile

from PyQt5.QtCore import QCoreApplication, QDate, QSettings
from PyQt5.QtWidgets import QDateEdit
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in InaSAFE, we were using from qgis.PyQt ..., isn't it?


def tr(self, string):
"""Translate string on processing context."""
return QCoreApplication.translate('Processing', string)
Copy link
Contributor

Choose a reason for hiding this comment

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

This would create another context in Transifex, why not using the normal tr already in InaSAFE?

string should be a unique, short, character only string, eg "qgis" or
"gdal". This string should not be localised.
"""
return 'geosys'
Copy link
Contributor

Choose a reason for hiding this comment

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

geosys?

within the GUI.
This string should be short (e.g. "Lastools") and localised.
"""
return 'GEOSYS'
Copy link
Contributor

Choose a reason for hiding this comment

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

geosys?

@myarjunar
Copy link
Collaborator Author

Haha thanks for the quick review @Gustry ! most of it are still copy and paste codes 😬 will definitely update it and put the actual code as we are moving forward cc @lucernae

@lucernae
Copy link
Collaborator

Thanks @myarjunar @Gustry I will take over the PR

@Gustry
Copy link
Contributor

Gustry commented Oct 31, 2019

@myarjunar I was so excited to see a PR with Processing ;-) And then i noticed it was only a template.

Good work @lucernae. Processing is a framework, so it's not easy sometimes to fit in this framework.

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.

None yet

3 participants