Skip to content

Conversation

@MattZur
Copy link
Collaborator

@MattZur MattZur commented Apr 1, 2025

This PR adds markdown to the front page to describe how to use the software and how to get started.

@MattZur MattZur requested a review from jwaiton April 1, 2025 14:16
@MattZur MattZur self-assigned this Apr 1, 2025
@MattZur MattZur added the documentation Improvements or additions to documentation label Apr 1, 2025
Copy link
Member

@jwaiton jwaiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work! While it looks like there a lot of comments, they're mostly just clarifying things (or pushing you to try out some of the other github features that are worth learning how to use).

The only two main takeaways I'd have would be

  • Try to keep instructions more general. While the current ones apply to how the repository works at this moment in time, it will (hopefully) do more than what it's capable of atm. Example being: proc is currently the only 'pack', but there will be more!
  • I think Getting Started should be above Usage. I know you didn't choose this (it was setup in that structure by me whoops) but it flows better if Getting Started comes first.

Once again, nice work! 😸

README.md Outdated
## Usage

Implement description of full usage, perhaps move this section to docs.
Usage is currently focused on binary files for readout from the above named systems.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it hasn't been referenced anywhere, I would specify that the software used for data acqusition is WaveDump 1 and 2. Possibly I'd even change the 'What is MULE section' first bulletpoint to highlight that, instead of doing it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph brings up a good point, currently the only usage is data processing, perhaps there should be a 'roadmap' of basic features desired in MULE that we can work towards.

If you want, you can open your first issue addressing this :)

README.md Outdated
Implement description of full usage, perhaps move this section to docs.
Usage is currently focused on binary files for readout from the above named systems.

After data collection and having saved the .bin files into the desired directory, one should copy the file path into the config file using a text editor (such as vim). In addtion, the destination file path and name should be enetered in the save_path line. Finally, ensure to have the correct wavedump edition as that will affect decoding.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo enetered -> entered.

We don't just have .bin files at the moment, we also have .dat (for WD1 processing). I'd keep the filetype vague, instead just 'the input file' or something along those lines. This means if we add further filetypes in later the sentence will still apply.

Copy link
Member

@jwaiton jwaiton Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MULE is made to be run anywhere on your system by design, once you've run the setup.sh you don't need to move the data into the config files, you can process them wherever they end up, as long as the paths in the config are correct.

Likewise, the configs can be copied and pasted wherever you so wish, the ones provided are so the user knows what inputs are required to make the configs work.

README.md Outdated

This will generate the .h5 file and store them in the predefined location.

In addition, there are several processing funcitons which are not yet run automatically from the config file but that can be found in /MULE/packs/proc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another good point! I'd possibly add subheaders (in markdown you increase the hashtags) for 'running MULE' and 'auxilary tools' or something along those lines.

In the future we should add a link to the automated documentation of these functions found here at the moment.

README.md Outdated

To clone the directory one should install git from the terminal with something like pip:

`pip install git`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip is a package manager specifically supplied for python. Installing git on different machines differs quite a bit. For the sake of sanity, lets remove this and hope the user knows how to install packages on their machine :)

README.md Outdated
Comment on lines 42 to 58
The simplest method of installation is one done by cloning the github repository and running the setup.sh file from the terminal, as explained below.

To clone the directory one should install git from the terminal with something like pip:

`pip install git`

then, one can clone the repo to the desired directory by using,

`git clone https://github.com/nu-ZOO/MULE.git`

> **_NOTE_** The above link is taken from the repo's github page
The environemnt then needs to be created and activated. This can be achieved by running the following command from the MULE directory:

`source setup.sh`

One can also create an alias to activate the environment easily on every session.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first paragraph is perfect, but I would suggest to provide all the commands a user would have to type in a code block, like so:

git clone https://github.com/nu-ZOO/MULE.git
cd MULE
source setup.sh 

Don't worry about informing the user about what each step does, the 'basic installation' should just be answering the question: 'I don't care what it means, I just want MULE to start running'.

The comment on the alias is also good :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, possibly warn them about two things:

  • Currently setup.sh only works as expected on x86_64 machines. This will be fixed soon.
  • This will try to install miniconda on your machine.

@MattZur
Copy link
Collaborator Author

MattZur commented Apr 2, 2025

@jwaiton Thanks so much for the feedback. That's all good, just got a question about the Usage section. Would it work fine to keep mule proc process_WD2_3channel.conf by stating that it is an example and that choice of pack will be available, or is there some pseudocode that can be used instead?

@jwaiton
Copy link
Member

jwaiton commented Apr 2, 2025

@jwaiton Thanks so much for the feedback. That's all good, just got a question about the Usage section. Would it work fine to keep mule proc process_WD2_3channel.conf by stating that it is an example and that choice of pack will be available, or is there some pseudocode that can be used instead?

I think its fine to keep the mule proc process_WD2_3channel.conf part. It's a good example (and one of the few that works at the moment :) ). It's also a good line to demonstrate the general template of mule <PACK> <CONFIG>.

Copy link
Member

@jwaiton jwaiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrapping up nicely! Just a minor comment and some typos then its ready to go!

README.md Outdated
Usage is currently focused on binary files for readout from the above named systems.

## Getting Started
After data collection and having saved the unprocessed files, one should copy the file path into the config file using a text editor (such as vim). In addtion, the destination file path and name should be enetered in the save_path line. Finally, ensure to have the correct wavedump edition as that will affect decoding.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo of enetered -> entered

README.md Outdated
EXPLAIN SIMPLEST METHOD FOR INSTALLATION/SETUP
## MULE Auxilary Tools

There are several additonal processing funcitons which are not yet run automatically from the config file but that can be found in /MULE/packs/proc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo of funcitons -> functions

README.md Outdated
Usage is currently focused on binary files for readout from the above named systems.

## Getting Started
After data collection and having saved the unprocessed files, one should copy the file path into the config file using a text editor (such as vim). In addtion, the destination file path and name should be enetered in the save_path line. Finally, ensure to have the correct wavedump edition as that will affect decoding.
Copy link
Member

@jwaiton jwaiton Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be pedantic, but I think this can be explained in a more general manner..

Something along the lines of
'To process a file with one of MULE's packs, you define a config with the required input parameters (such as input file).'

The next line about where to find the template config files is perfect, as it points the user to where they need to go to see all the parameters for any particular process.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'll remove this paragraph and put something more general. But do you think it's worth keeping the line "the destination file path and name should be entered in the save_path line"? perhaps it should go as a comment on the template config file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be general documentation for what the config parameters are for each pack, but I think that's not a necessity for the 'getting started' section. That'll be a problem further down the line :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely, thanks!

@MattZur
Copy link
Collaborator Author

MattZur commented Apr 2, 2025

Should the commit message be "fix typos and make ... "? Not sure if this was only for PR's or commits too

@jwaiton
Copy link
Member

jwaiton commented Apr 2, 2025

Should the commit message be "fix typos and make ... "? Not sure if this was only for PR's or commits too

Commits should be imperative, the same as the PR title.
So:

fix typo and make...

is good.

do a thing

is the general structure for commits and PRs. Issues are less strict (although these are all just generic rules for keeping things tidy).

Copy link
Member

@jwaiton jwaiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR enhances the README.md to include information on running MULE, while staying sufficiently general enough that it shouldn't require any changes for a while. Good job!

@MattZur MattZur merged commit abdcde4 into nu-ZOO:main Apr 2, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants