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

Normalise.py changes for MACE use #333

Conversation

digitensions
Copy link

@digitensions digitensions commented Apr 18, 2019

Ran a 2to3 pass and tested here at MACE. Working great! Added missing parenthesis at line 123, and amendment to slices - now 24. Not sure if you'll want to replace your own with this, so should I change slices back to 16?

Woohoo. Made an IFIscripts pull request though :) thanks for suggesting Kieran.

To be tested
Updated code to working version following 2to3 update, and edits to lines 123 and 83.  This code works perfectly on home Mac, needs further testing on MACE macs.
@digitensions
Copy link
Author

Hang on, had a fail at the end on my MacPro with this code although it works fine on laptop... reviewing

@digitensions
Copy link
Author

digitensions commented Apr 18, 2019

Managed to get it working okay on Mac Pro at office, and everything going through fine, except it's not carrying the name of the original file across to the files generated, instead it's like a 24 character alpha-numeric jumble. More work needed for MacPro v3.7.3, but working fine on MacBook v3.7.2 of Python.

@kieranjol
Copy link
Owner

kieranjol commented Apr 18, 2019 via email

@kieranjol
Copy link
Owner

Hi Joanna, the actual 2to3 stuff looks great and I think that the bump up to 24 slices is a good idea. Maybe I/someone could add an argparse argument to specify the slice count maybe?

Also is there any chance that you were using an older version of normalise.py? I see a bunch of deletions in the diff towards the end of your Pull Request, which make me think that you didn't use the latest version in the master branch?

@digitensions
Copy link
Author

Hi Kieran, It's very possible I thought I'd downloaded the latest version when it was an original. So sorry! Can you send a link to where I download from when you get time? Just so I can be sure. Thanks :)

@kieranjol
Copy link
Owner

kieranjol commented Apr 25, 2019 via email

@digitensions
Copy link
Author

Well I'm pretty sure that's where I downloaded it from, twice. I'll try again and compare the code to the Master branch to be sure :)

@kieranjol
Copy link
Owner

kieranjol commented Apr 26, 2019 via email

@digitensions
Copy link
Author

digitensions commented May 10, 2019

Hey, well I've managed to resolve the issue on MACE's Mac. Despite replicating the alterations on MACE MacPro, the only way to fix it was to copy the working normalise/ififuncs across from my laptop to the MACE Mac... I'm checking the codes against one another and they look identical at a swift glance!! Should I do anything to check this working normalise.py version (slices 24) against your script or do you just want to burnnnn this wasted PR!

@digitensions
Copy link
Author

Today I discovered I can close things, so shutting this one down and considering it a success. Thanks for help!

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

2 participants