search

EGaraldi / corecon

User-friendly collection of Reionization-related constraints
https://corecon.readthedocs.io/en/latest/
GNU General Public License v3.0
8 stars 5 forks source link

Typos in tutorial (documentation) #4

Closed Stefan-Heimersheim closed 2 years ago

Stefan-Heimersheim commented 4 years ago

Hello,

in the example in the docs you seem to be mixing up the IGM temperature and ionized fraction:

#get IGM temperature at mean density
ionfr = crc.get("T0")
[...]
fig.savefig( "neutral_fraction_evolution.png" , bbox_inches='tight')

Just change it to ionized fraction there:

ionfr = crc.get("ionized_fraction")

Also this line is missing the 'all' (or whatever the first attribute should be) in the tutorial:

    ionfr[k].nan_to_values('all', 0.1)

Best, Stefan

Stefan-Heimersheim commented 4 years ago

I just noticed another potential typo. The line

ionfr[k].values = 1-ionfr[k].values

is quite confusing and also rewrites the dictionary (not the internal one but ionfr). It should instead be renamed to neutralfr or similar so people who start based of the tutorial don't get confused.

EGaraldi commented 2 years ago

Dear Stefan, please accept my apologies for the long silence. I had some personal issues in 2020 that forced me to focus away from this project. I then had to find a new job, and by the time this was done the JOSS submission was closed for inactivity, so I drifted away from this project. I have now finally restarted working on this project, so you can expect a (much much) quicker response from now on.

Coming to your comment, I have fixed the typos in the tutorial (I had forgotten to trigger a new build of the docs), while for the re-naming of ionfr[k].values I think this aligns with the philosophy of providing a copy of the data dict. and of functions like swap_limits and nan_to_value, i.e. to provide an object that can be freely manipulated while retaining the original data. I am not sure this is the best approach for everyone, but I prefer to keep the tutorial aligned with the design philosophy. I have added a comment stressing that ionfr[k].values now contains the neutral fraction.