testing if datamaps v1.0.0 #4

Closed
opened 2019-11-21 17:35:08 +00:00 by banillie · 17 comments
banillie commented 2019-11-21 17:35:08 +00:00 (Migrated from github.com)

Hi Matt, started some testing of v1.0.0... did a master to template command... which raised some critical issues in relation to keys in the master not being present in the dm. Great functionality. However, it this shouldn't be seen as a critical issue that stops the command, because in the instance of master to template, there will (there should be the option to) be some key/values that the user doesn't want to move across to the template in order to prevent any formulas present in the template being over written.... we manage this process via tagging and filter keys in our masters dm document. e.g. if a key is relating to a value that is automatically updated in the template we mark it as auto and then filter it out when building the dm cvs file for master to template export. Could this check be relegated to below critical? cheers.

Hi Matt, started some testing of v1.0.0... did a master to template command... which raised some critical issues in relation to keys in the master not being present in the dm. Great functionality. However, it this shouldn't be seen as a critical issue that stops the command, because in the instance of master to template, there will (there should be the option to) be some key/values that the user doesn't want to move across to the template in order to prevent any formulas present in the template being over written.... we manage this process via tagging and filter keys in our masters dm document. e.g. if a key is relating to a value that is automatically updated in the template we mark it as auto and then filter it out when building the dm cvs file for master to template export. Could this check be relegated to below critical? cheers.
yulqen commented 2019-11-21 20:42:39 +00:00 (Migrated from github.com)

Hi Will,

Exactly the kind of feedback needed so thanks. Will take a look tomorrow and update.

Pushed a couple of PRs your way tonight. Nothing major.

M

--
Matthew Lemon
matt@matthewlemon.com

On Thu, 21 Nov 2019, at 5:35 PM, Will Grant wrote:

Hi Matt, started some testing of v1.0.0... did a master to template
command... which raised some critical issues in relation to keys in the
master not being present in the dm. Great functionality. However, it
this shouldn't be seen as a critical issue that stops the command,
because in the instance of master to template, there will (there should
be the option to) be some key/values that the user doesn't want to move
across to the template in order to prevent any formulas present in the
template being over written.... we manage this process via tagging and
filter keys in our masters dm document. e.g. if a key is relating to a
value that is automatically updated in the template we mark it as auto
and then filter it out when building the dm cvs file for master to
template export. Could this check be relegated to below critical?
cheers.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/hammerheadlemon/datamaps/issues/4?email_source=notifications&email_token=AAB4ISJUHZPLJWTMRWYDNILQU3BEZA5CNFSM4JQFZPZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H3FOBEQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4ISMLI7MEECLYS6GZEWLQU3BEZANCNFSM4JQFZPZQ.

Hi Will, Exactly the kind of feedback needed so thanks. Will take a look tomorrow and update. Pushed a couple of PRs your way tonight. Nothing major. M -- Matthew Lemon matt@matthewlemon.com On Thu, 21 Nov 2019, at 5:35 PM, Will Grant wrote: > Hi Matt, started some testing of v1.0.0... did a master to template > command... which raised some critical issues in relation to keys in the > master not being present in the dm. Great functionality. However, it > this shouldn't be seen as a critical issue that stops the command, > because in the instance of master to template, there will (there should > be the option to) be some key/values that the user doesn't want to move > across to the template in order to prevent any formulas present in the > template being over written.... we manage this process via tagging and > filter keys in our masters dm document. e.g. if a key is relating to a > value that is automatically updated in the template we mark it as auto > and then filter it out when building the dm cvs file for master to > template export. Could this check be relegated to below critical? > cheers. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <https://github.com/hammerheadlemon/datamaps/issues/4?email_source=notifications&email_token=AAB4ISJUHZPLJWTMRWYDNILQU3BEZA5CNFSM4JQFZPZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H3FOBEQ>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB4ISMLI7MEECLYS6GZEWLQU3BEZANCNFSM4JQFZPZQ>. >
banillie commented 2019-11-22 10:05:23 +00:00 (Migrated from github.com)

Great cheers Matt. Yes starting to do more testing and prep for Q3 so should hopefully throw up some useful stuff.

Thanks for the PR will take look.

Great cheers Matt. Yes starting to do more testing and prep for Q3 so should hopefully throw up some useful stuff. Thanks for the PR will take look.
yulqen commented 2019-11-22 17:09:56 +00:00 (Migrated from github.com)

Just to clarify, you said "keys in the master not being present in the dm". Do you actually mean the other way round - there is a key in the datamap which doesn't appear in the master? Only in the latter case does this CRITICAL error currently get raised as far as I can tell (github.com/hammerheadlemon/bcompiler-engine@3d05209777/engine/use_cases/output.py (L71))

If I remember correctly, the thinking was that the user manages the datamap, using filters and hiding rows, etc, to only make available those values in the datamap that should be exported (as you say in your issue). As long as he/she doesn't leave an entry in the datamap which does not appear in the master, the export should take place.

Are you saying that you would prefer it if datamaps skipped entries it finds in the datamap if a corresponding value isn't present in the master to be exported? I'm slightly confused in which case because I thought that was the purpose of filtering in the datamap. It would be dangerous to do it any other way as it is the datamap that should be the single version of the truth, not a master file. I'm obviously getting the wrong end of the stick here - maybe a chat would solve at some point!

Otherwise, if you could you encrypt and send me the files you are using to trigger this I could probably work it out...

Just to clarify, you said "keys in the master not being present in the dm". Do you actually mean the other way round - there is a key in the **datamap** which doesn't appear in the **master**? Only in the latter case does this `CRITICAL` error currently get raised as far as I can tell (https://github.com/hammerheadlemon/bcompiler-engine/blob/3d05209777ed9a4d63adf5c71e124a4e1e5bc995/engine/use_cases/output.py#L71) If I remember correctly, the thinking was that the user manages the datamap, using filters and hiding rows, etc, to only make available those values in the datamap that should be exported (as you say in your issue). As long as he/she doesn't leave an entry in the datamap which does not appear in the master, the export should take place. Are you saying that you would prefer it if datamaps skipped entries it finds in the datamap if a corresponding value isn't present in the master to be exported? I'm slightly confused in which case because I thought that was the purpose of filtering in the datamap. It would be dangerous to do it any other way as it is the datamap that should be the single version of the truth, not a master file. I'm obviously getting the wrong end of the stick here - maybe a chat would solve at some point! Otherwise, if you could you encrypt and send me the files you are using to trigger this I could probably work it out...
banillie commented 2019-11-23 10:01:29 +00:00 (Migrated from github.com)

The testing I did seemed to raise an error when the key was present in the DM, but not the master. Not the other way around.

I agree with what you say in relation to user requirements. It should raise a critical error if key present in DM but not in master.

I’ll try some further testing to double check. Will prob also send over a set of test docs, so you can try and replicate.

Cheers, Will.

The testing I did seemed to raise an error when the key was present in the DM, but not the master. Not the other way around. I agree with what you say in relation to user requirements. It should raise a critical error if key present in DM but not in master. I’ll try some further testing to double check. Will prob also send over a set of test docs, so you can try and replicate. Cheers, Will.
yulqen commented 2019-11-23 10:12:04 +00:00 (Migrated from github.com)

Ok. So if the key is in the datamap and not in the master, then the CRITICAL error is correct. The export cannot continue if you are asking it (via the datamap) to find a value in the master that isn't there - we should assume at all times that the datamap is correct. Hence your well thought-out facility to hide rows in the datamap. Hope this is right.

Happy to chat this weekend.

Ok. So if the key is in the datamap and not in the master, then the CRITICAL error is correct. The export cannot continue if you are asking it (via the datamap) to find a value in the master that isn't there - we should assume at all times that the datamap is correct. Hence your well thought-out facility to hide rows in the datamap. Hope this is right. Happy to chat this weekend.
banillie commented 2019-11-23 13:23:18 +00:00 (Migrated from github.com)

Sorry in the master and not in the DM. That’s what I believe is raising the error. I can test again tonight and confirm.

Sorry in the master and not in the DM. That’s what I believe is raising the error. I can test again tonight and confirm.
yulqen commented 2019-11-23 14:25:30 +00:00 (Migrated from github.com)

Ok, let me know dude.

Ok, let me know dude.
banillie commented 2019-11-23 22:34:40 +00:00 (Migrated from github.com)

Hi Matt, ran the test again and was not able to replicate the error message and I got datamaps to work so apologies for false alarm!

However, when checking the templates that were created, data/information is being placed into the wrong cells in the templates. I’ve checked the DM to see whether the wrong key cell reference was provided but it looks right. So it might be that datamaps is not transferring properly? Sorry about this. Hopefully it’s me being stupid again!

I’ve sent the documents to your MR Lemon account so you can do testing also.

Thanks, Will.

Hi Matt, ran the test again and was not able to replicate the error message and I got datamaps to work so apologies for false alarm! However, when checking the templates that were created, data/information is being placed into the wrong cells in the templates. I’ve checked the DM to see whether the wrong key cell reference was provided but it looks right. So it might be that datamaps is not transferring properly? Sorry about this. Hopefully it’s me being stupid again! I’ve sent the documents to your MR Lemon account so you can do testing also. Thanks, Will.
yulqen commented 2019-11-24 16:13:28 +00:00 (Migrated from github.com)

Ok Will - thanks. Will take a look!

On 23-11-2019, Will Grant wrote:

Hi Matt, ran the test again and was not able to replicate the error message and I got datamaps to work so apologies for false alarm!

However, when checking the templates that were created, data/information is being placed into the wrong cells in the templates. I’ve checked the DM to see whether the wrong key cell reference was provided but it looks right. So it might be that datamaps is not transferring properly? Sorry about this. Hopefully it’s me being stupid again!

I’ve sent the documents to your MR Lemon account so you can do testing also.

Thanks, Will.

--
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub:
https://github.com/hammerheadlemon/datamaps/issues/4#issuecomment-557839086

--

Matthew Lemon
Email: matt@matthewlemon.com

Ok Will - thanks. Will take a look! On 23-11-2019, Will Grant wrote: >Hi Matt, ran the test again and was not able to replicate the error message and I got datamaps to work so apologies for false alarm! > >However, when checking the templates that were created, data/information is being placed into the wrong cells in the templates. I’ve checked the DM to see whether the wrong key cell reference was provided but it looks right. So it might be that datamaps is not transferring properly? Sorry about this. Hopefully it’s me being stupid again! > >I’ve sent the documents to your MR Lemon account so you can do testing also. > >Thanks, Will. > >-- >You are receiving this because you were assigned. >Reply to this email directly or view it on GitHub: >https://github.com/hammerheadlemon/datamaps/issues/4#issuecomment-557839086 -- Matthew Lemon Email: matt@matthewlemon.com
yulqen commented 2019-11-24 16:57:47 +00:00 (Migrated from github.com)

Oh dear - the current implementation of export relies on col A of the datamap being the same as col A in ther master, which is obviously not fit for purpose at all. Clearly you need to be able to adapt the datamap file to your needs (by hiding/omitting rows, as discussed in the comments above), and rely on the key names matching for a value to be exported.

What's your timescale to get this fixed?? This is obviously crucial and I will prioritise the fix.

Oh dear - the current implementation of `export` relies on col A of the datamap being the same as col A in ther master, which is obviously not fit for purpose at all. Clearly you need to be able to adapt the datamap file to your needs (by hiding/omitting rows, as discussed in the comments above), and rely on the key names matching for a value to be exported. **What's your timescale to get this fixed??** This is obviously crucial and I will prioritise the fix.
banillie commented 2019-11-24 20:09:18 +00:00 (Migrated from github.com)

Not massively urgent Matt. We have until 13 Dec for our commission. Although it would be very handy to be able to run tests to ensure the template is correct earlier than that.

Could we revert to an earlier version in the meantime? That would be useful.

Cheers. As always thanks for everything on this.

Will.

Not massively urgent Matt. We have until 13 Dec for our commission. Although it would be very handy to be able to run tests to ensure the template is correct earlier than that. Could we revert to an earlier version in the meantime? That would be useful. Cheers. As always thanks for everything on this. Will.
banillie commented 2019-11-25 11:47:01 +00:00 (Migrated from github.com)

Hi Matt, is it possible to revert to an earlier version of datamaps, while this is being fixed? Just wondering if possible way to proceed.
Cheers.

Hi Matt, is it possible to revert to an earlier version of datamaps, while this is being fixed? Just wondering if possible way to proceed. Cheers.
yulqen commented 2019-11-25 14:33:50 +00:00 (Migrated from github.com)

Fix included in datamaps 1.0.1 - can you please try using pip-based installation first before I create the .exe file?

Set up a test virtualenv using Command Prompt

* cd C:\Users\standalone\Desktop
* python -m venv tmp-venv [or 'python3' here]
* tmp-venv\Scripts\activate.bat

Install datamaps

* pip install datamaps

Test whether bcompiler-engine has also been updated (pip list - both datamaps and bcompiler-engine should be v1.0.1). If not, pip install bcompiler-engine.

Run the test

* datamaps export master C:\PATH\TO\master_commission_test.xlsx`

If all goes ok, you can delete the Desktop\tmp-venv directory to remove the test virtualenv.

The export code in bcompiler-engine is the least tested and robust at this point unfortunately, so your continued testing/patience here is absolutely invaluable! I hope the above goes without a hitch. Report back with issues. If you're happy, I will release the exe on the site.

Fix included in `datamaps 1.0.1` - can you please try using pip-based installation first before I create the .exe file? ### Set up a test virtualenv using Command Prompt ```bash * cd C:\Users\standalone\Desktop * python -m venv tmp-venv [or 'python3' here] * tmp-venv\Scripts\activate.bat ``` ### Install `datamaps` ```bash * pip install datamaps ``` Test whether `bcompiler-engine` has also been updated (`pip list` - both `datamaps` and `bcompiler-engine` should be `v1.0.1`). If not, `pip install bcompiler-engine`. ### Run the test ```bash * datamaps export master C:\PATH\TO\master_commission_test.xlsx` ``` If all goes ok, you can delete the `Desktop\tmp-venv` directory to remove the test virtualenv. The `export` code in `bcompiler-engine` is the least tested and robust at this point unfortunately, so your continued testing/patience here is absolutely invaluable! I hope the above goes without a hitch. Report back with issues. If you're happy, I will release the exe on the site.
banillie commented 2019-11-25 15:58:18 +00:00 (Migrated from github.com)

Hi Matt, I followed the instructions above and ran the export command. I got a the following

ModuleNotFoundError: No module named 'dataclasses'

cheers.

Hi Matt, I followed the instructions above and ran the `export` command. I got a the following > ModuleNotFoundError: No module named 'dataclasses' cheers.
yulqen commented 2019-11-25 16:24:15 +00:00 (Migrated from github.com)

Ok - this is the python 3.6 thing again. Scrap the idea of installing using the above - I'm not going to ask you to install Python 3.7 just now after putting you through that last time.

Just download the new dm.exe from https://datamaps.twentyfoursoftware.com/ and use that to test please (it should be v1.0.1). Sorry Will - I need to trap this problem about what Python version is being used by pip. I will also remove the use of dataclasses in bcompiler-engine because this is a 3.7 and above feature. None of this matters for users who download the exe because they get Python 3.7 bundled with it.

Let me know how you get on.

Ok - this is the python 3.6 thing again. Scrap the idea of installing using the above - I'm not going to ask you to install Python 3.7 just now after putting you through that last time. Just download the new `dm.exe` from https://datamaps.twentyfoursoftware.com/ and use that to test please (it should be v1.0.1). Sorry Will - I need to trap this problem about what Python version is being used by pip. I will also remove the use of `dataclasses` in `bcompiler-engine` because this is a 3.7 and above feature. None of this matters for users who download the exe because they get Python 3.7 bundled with it. Let me know how you get on.
banillie commented 2019-11-25 16:38:42 +00:00 (Migrated from github.com)

Have run and test and looks all good Matt. Data seems to be transferring nicely. 🐍 👍 💯

Will do some more testing over the coming days.

Have run and test and looks all good Matt. Data seems to be transferring nicely. 🐍 👍 💯 Will do some more testing over the coming days.
yulqen commented 2019-11-25 16:46:06 +00:00 (Migrated from github.com)

Magic. Closing this now. Please continue to open issues as necessary!

Magic. Closing this now. Please continue to open issues as necessary!
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
yulqen/datamaps#4
No description provided.