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

Change loadinfrastructure #1820

Merged
merged 23 commits into from
Dec 21, 2018
Merged

Change loadinfrastructure #1820

merged 23 commits into from
Dec 21, 2018

Conversation

LePetitTim
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Dec 18, 2018

Coverage Status

Coverage increased (+0.08%) to 89.907% when pulling 3cae4a8 on change_loadinfrastructure into 923df13 on master.

@LePetitTim LePetitTim force-pushed the change_loadinfrastructure branch 2 times, most recently from a35e94b to 3c13ff6 Compare December 18, 2018 20:57
@LePetitTim LePetitTim force-pushed the change_loadinfrastructure branch 2 times, most recently from 01b4a0e to 3b551dd Compare December 19, 2018 08:42
try:
year = int(feature.get(field_implantation_year))
except ValueError:
year = self.get_year_from_date(feature.get(field_implantation_year))
Copy link
Contributor

Choose a reason for hiding this comment

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

C'est trop spécifique à un cas particulier. C'est me genre de choses pour lequelles il faut prétraiter le fichier source en extrayant l'année dans une nouvelle colonne (en utilisant QGis ou autre) avant de l'importer.


def get_year_from_date(self, year):
year_from_date = year.split('/')[-1]
return int(year_from_date)
Copy link
Contributor

Choose a reason for hiding this comment

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

A supprimer car trop spécifique à un cas particulier.

elif options.get('condition_default'):
condition = options.get('condition_default')
else:
condition = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Le principe de .get() est de renvoyer None si la clé n'existe pas. Il y a donc moyen de simplifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a oui
en effet

description=description,
implantation_year=year
)
fields_withtout_eid = {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: withTout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


if field_condition_type in available_fields:
condition = feature.get(field_condition_type)
elif options.get('condition_default'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Il faut un simple else. Sinon la variable condition ne sera pas définie dans certains cas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

condition = feature.get(field_condition_type)
else:
condition = options.get('condition_default')
structure = Structure.objects.get(feature.get(field_structure_type)) \
Copy link
Member

Choose a reason for hiding this comment

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

attention a ne pas faire de .get (requete django) sans mettre un nom de colonne à matcher, unique de surcroit

Copy link
Member

Choose a reason for hiding this comment

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

en fait tu recuperes deja la structure plus haut, le Structure.objects.get ne sert pas la, si ?

Copy link
Member

Choose a reason for hiding this comment

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

y'a un try/except fait pour plus haut

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Le premier c'est pour la valeur par default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Il sert a recuperer uniquement la structure dans le fichier .shp avec --structure-field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

par contre il risque d'avoir un probleme normalement cette ligne elle est faite pour mettre la structure par default si la structure dans le fichier n'existe pas
.

Copy link
Member

@submarcos submarcos Dec 20, 2018

Choose a reason for hiding this comment

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

ok par contre faut pas faire de .get si tu mets pas de parametre(s) de matching unique(s)
à moins que tu cherches à recuperer la seule entrée de la base, mais dans ce cas tes parametres de lookup ne servent à rien)

et c'est un peu dommage de s'occuper de la structure à deux endroits différents

pareil, y'a des conditions gérées en oneline c'est bien, autant le faire sur toutes. voila fixe ces points et je merge

Copy link
Member

Choose a reason for hiding this comment

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

@submarcos submarcos merged commit 084f172 into master Dec 21, 2018
@submarcos submarcos deleted the change_loadinfrastructure branch December 21, 2018 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants