Closed fherwig closed 8 years ago
@fherwig Why do you create a pull request for something that is not intended to be merged? Wouldn't a branch and an issue suffice?
@fherwig why is this method a good choice for comparing line charts? If things are offset by just one pixel, entropy could be huge this way ...
@fherwig @adam-paul @korobkin abu_chart.py should use tempfile.TemporaryDirectory to create a directory to put downloaded files in, rather than putting them into the local directory. The same should be done for all test data generated by the test.
@2sn this branch may or may not be merged, but this is a reasonable way of using a pull request
@2sn I am demonstrating how this method works in practice - have you tried it? the example compares for a sequence of abu_chart plots subsequent images. if you compare that output plot with the image sequence you can clearly see that this method along with a suitable limiter separates image differences into two groups, one which can be considered significant changes, and one that could be considered insignificant changes
abu_chart.py should use tempfile.TemporaryDirectory to create a directory to put downloaded files in, rather than putting them into the local directory. The same should be done for all test data generated by the test.
@2sn I don't know how to do that - could you implement that for this example and push back? I am keen to learn.
@fherwig
I am demonstrating how this method works in practice - have you tried it? the example compares for a sequence of abu_chart plots subsequent images. if you compare that output plot with the image sequence you can clearly see that this method along with a suitable limiter separates image differences into two groups, one which can be considered significant changes, and one that could be considered insignificant changes
I think for this test you'd rather want to compare numerical values rather than plotting them and then compare!
@fherwig
I don't know how to do that - could you implement that for this example and push back? I am keen to learn.
don't have time now - I leave this for your student to try out, should not take more than a few min to an hour ...
@fherwig how do I call this test, is there an example?
@2sn the plot is just a demonstration, a visual aid to see what is going on. for sure in the actual test we should use only numerical values, that's the whole idea; there is a README.md file that explains how it is used. should use out of the box, please try.
@adam-paul can you implement this tempfile.TemporaryDirectory
method @2sn mentioned?!
OK, you need to go to the testing
dir on this branch and check the README.md file
@fherwig in the README.md file you write
export PYTHONPATH=$PYTHONPATH:"/path/to/NuGridPy"
python abu_chart.py
python compare_image_entropy.py
it seem you have not embraced the concept of the NuGridPy module.
what you write will
@fherwig
OK, you need to go to the testing dir on this branch and check the README.md file
for use of a NuGridPy as a module, you do not want to be in a (sub-)directory of the module.
@2sn please update the wiki so that everybody can easily get up to speed with the concept of the NuGridPy module. Just criticizing without trying to be helpful puts people off. So please just try and be a bit more constructive. Thanks.
for use of a NuGridPy as a module, you do not want to be in a (sub-)directory of the module.
why not?
@fherwig
why not?
the module import machinery can get confused, depending on the order of directories in your path.
@fherwig @adam-paul
as for tempfile.TemporaryDirectory
you need to run the download and test for the same file such that the tempdir object is not lost between the two calls (and the data hence deleted).
@fherwig @adam-paul added use for temp file, updated README.md, python3
@fherwig The basic paradigm is to treat NuGridPy like a standard module installed on your system, e.g., numpy.py: you would not seek out where it is installed on the system, change to some sub-directory in there and download files into it. You just load the module, sub-modules, import objects (functions, classes, ...) from there. But you would not run things in that directory.
On 15/06/16 17:04, Falk Herwig wrote:
@2sn https://github.com/2sn please update the wiki so that everybody can easily get up to speed with /the concept of the NuGridPy module./ Just criticizing without trying to be helpful puts people off. So please just try and be a bit more constructive. Thanks.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NuGrid/NuGridPy/pull/12#issuecomment-226216056, or mute the thread https://github.com/notifications/unsubscribe/ABNtFrGAy1qwtKl19KYCXZr7Ukgu4RVuks5qMBRogaJpZM4I2aYU.
@fherwig @adam-paul does the revised version work for you? there has also been some upgrades and a bug fix for core modules. @korobkin How do we go about bug fixes to core modules (w/r python3 and otherwise) that we come across while working on branches?
@korobkin @adam-paul @2sn @swjones
Please have a look in the
testing
dir on this branch and check the README.md file. This branch is not necessarily meant to be directly merged, but rather demonstrates two techniques that maybe useful for the NuGridPy tests:@adam-paul - I think these are tests that could, be implemented in travis?