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

[IMP] cfdilib: Removed automatic NA assignation to empty values #74

Merged
merged 1 commit into from
Jan 8, 2018

Conversation

luistorresm
Copy link
Collaborator

@luistorresm luistorresm commented Jan 8, 2018

CFDI 3.3 have a extensive catalog, then when is validated the xsd all
the attributes are validated that the value assigned is found in the
catalogs.

Before of this change, if the attribute in the dict values was
empty, this was updated to set NA, but in CFDI 3.3, is validated that
the value exist in the catalog, and take many time in compare the value.

Example in the attribute 'ClaveProdServ' that have 52828 records.

Now, if the value is empty only not add the attribute and do not take
many time because only validate that the attribute is not found.

Why not only add a validation like if attribute '!=' 'NA', because the
catalog to Unit Codes have the record NA - Miligramo por kilogramo and
We could ignore that value.

Here a time:
captura de pantalla 2018-01-08 a la s 11 10 37

CFDI 3.3 have a extensive catalog, then when is validated the xsd all
the attributes are validated that the value assigned is found in the
catalogs.

Before of this change, if the attribute in the dict values was
empty, this was updated to set NA, but in CFDI 3.3, is validated that
the value exist in the catalog, and take many time in compare the value.

Example in the attribute 'ClaveProdServ' that have 52828 records.

Now, if the value is empty only not add the attribute and do not take
many time because only validate that the attribute is not found.

Why not only add a validation like `if attribute '!=' 'NA'`, because the
catalog to Unit Codes have the record `NA - Miligramo por kilogramo` and
We could ignore that value.
@luistorresm
Copy link
Collaborator Author

@nhomar

Could you merge this PR, please?

Regards

@nhomar nhomar merged commit 5b97415 into Vauxoo:master Jan 8, 2018
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