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

Project structure #146

Merged
merged 24 commits into from
Mar 6, 2024
Merged

Conversation

Jerakin
Copy link
Contributor

@Jerakin Jerakin commented Feb 21, 2024

Summary

Attempt to modernize the project layout

Details

src layout

Moved python source and website files into a src/wikmd/ folder.

Added project.tom

Setting up a pyproject file. The file is commonly used to define project settings as well as
a central location for other tools settings.

Removed requirements.txt

The requirements now live in the pyproject file and the common pip install -r requirements.txt is now
replaced by doing pip install . to install from it. If you want the dev dependencies you can do
pip install .[dev].

Dockerfile

Moved Dockerfile into root as well as tried to simplify it. However this is not my area of expertise so needs extra care to make sure everything is working. But I have tested it extensively on my server (running Ubuntu) and it builds, installs and runs as expected.

Tests

I took the liberty to clean up the tests a bit, the functionality is the same of course.

wiki_template

As I wanted to get away from the the v6 docker setup I changed it so that the app populates the defined wiki_directory if it it is empty (or doesn't exist). That way we can make Docker image that works on all platform python works on.

Consider ruff

I think we should consider using ruff in the project as it is quickly becoming the way to ensure code consistency, however I haven't added it yet. I have added some settings and rules for the test folders. If you want to try it out you can install it with pip install ruff and installing the ruff plugin for your IDEA, however it might be a bit overwhelming as there is a lot of lint error.

Work left

  • No tests fails
  • Project runs locally as before
  • Docker image builds
  • Docker image installs
  • Project runs on a docker image as before

@Jerakin Jerakin mentioned this pull request Feb 21, 2024
5 tasks
@Jerakin Jerakin marked this pull request as ready for review February 21, 2024 22:56
@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 21, 2024

This is now ready for review. Let me know of any concerns you have with the Pull Request @Linbreux

@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 22, 2024

Is it too hard to review? Maybe you want it broken down in to a couple of PRs?

@Linbreux
Copy link
Owner

@Jerakin no it's fine like this. I just need to find some time to test it (because it's a big one). ;)

But anyway, thanks for your hard work!

@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 22, 2024

Forgot to mention: To run the app cd into src and run it as a module python -m wikmd.wiki make sure you got the -m and no .py.

Recommended way to run as a developer is to

Do once:

cd wikmd  
python -m venv \venv                     # Create a venv, it is required
venv\Scripts\activate                    # Activate the virtual env
python -m pip install .[dev] --editable  # Install the project in developer mode
python -m wikmd.wiki                     # Start the project

In the future you only need

venv\Scripts\activate                    # Activate the virtual env, skip if it's already active.
python -m wikmd.wiki                     # Start the project

@Linbreux Linbreux mentioned this pull request Feb 24, 2024
Copy link
Owner

@Linbreux Linbreux left a comment

Choose a reason for hiding this comment

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

This is a huge improvement. I have some extra questions/remarks:

  • Log file inside src folder.
  • Would it be possible to update the docs (installation)?
  • Image upload works. But the uploaded image can not be viewed. Image lookup is done in src folder.
  • Is it possible to run the python -m wikmd.wiki command one level higher. (not in the src folder)

Overal I think some stuff could be placed outside the src folder.

DOCKER.md Show resolved Hide resolved
src/wikmd/wikmd-config.yaml Outdated Show resolved Hide resolved
@Jerakin Jerakin force-pushed the refactor/project-structure branch from cad5bc5 to 12c73e5 Compare February 25, 2024 11:21
@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 25, 2024

Is it possible to run the python -m wikmd.wiki command one level higher. (not in the src folder)

Yes - but not without other "inconveniences". The recommended way to work on something that lives in the src folder is to install the project in edit mode.

If you are within the project (the wikmd folder) you can install it through python -m pip install --editable . this will install it as an editable (basically the installed project within pythons Lib/site-packages will point towards this folder). You can then do python -m wikmd.wiki to start it (from anywhere - as long as the active python interpreter is the correct one. However because we need files to be in the correct location you need to run it from a location that makes sense).

Overal I think some stuff could be placed outside the src folder.

Remember that the intent with the src folder is to be able to package the project, so everything that the app needs to be distributed as such would need to be placed within the src folder. With that in mind is there any changes you would like to see to the structure?

The wiki_template folder should live within the src folder too. Maybe we should move the wiki_template, static and templates into a subfolder within src named assets or something.

moving the template folder into source to indicate that it is part of the app proper
Ensure the app works if people use pip install . without the editable flag
@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 25, 2024

I moved the wiki template folder into the src to indicate that it's part of the app.

I made a few updates to make sure that we can build the app and have it run (you could at this point upload it to pypi if you wanted).

I changed and added documentation, including what the recommended way to run it as a developer is.

(I would guess that the docker image isn't 100% working with these fixes, will fix that of course)

@Linbreux
Copy link
Owner

If you are within the project (the wikmd folder) you can install it through python -m pip install --editable . this will install it as an editable (basically the installed project within pythons Lib/site-packages will point towards this folder). You can then do python -m wikmd.wiki to start it (from anywhere - as long as the active python interpreter is the correct one. However because we need files to be in the correct location you need to run it from a location that makes sense).

I think we can make that work. I was able to run it using this branch with:

cd wikmd
python -m pip install .
python -m wikmd.wiki

So I'm not quite sure why the editable flag is needed.

Remember that the intent with the src folder is to be able to package the project, so everything that the app needs to be distributed as such would need to be placed within the src folder. With that in mind is there any changes you would like to see to the structure?

I ment stuff like the config file. I think it would make more sense in the project root.

I moved the wiki template folder into the src to indicate that it's part of the app.

You could at this point upload it to pypi if you wanted

Nice thanks! Wanted to do that for a while.

@Jerakin
Copy link
Contributor Author

Jerakin commented Feb 25, 2024

So I'm not quite sure why the editable flag is needed.

the --editable flag is to enable development mode. In short - if you use --editable the installed package is pointing to your current files. So if you do any update it will instantly be reflected in the installed app (as they are the same). If you install the package without the editable flag then it is install as is, meaning if you want to do a change in the wiki.py you would also need to run python -m pip install . again.

In other words use python -m pip install . if you only want to use it and python -m pip install .[dev] --editable if you are a developer and want to improve it.

I ment stuff like the config file. I think it would make more sense in the project root.

Yeah that could be a good idea. I think we could go further, I would like to see changes to remove the config file completely from the project. As right now, it is almost as if we have default variables within config.py as well as the config.yaml file. Which can be confusing.

Personally I would like there to be support for a config.yaml file that the user can supply (through a Environment variable), however the project shouldn't depend on one existing (as it does now) and if there isn't a config file we use the default variables from config.py.

@Jerakin Jerakin requested a review from Linbreux February 26, 2024 10:53
@Linbreux Linbreux merged commit b87ae28 into Linbreux:main Mar 6, 2024
6 checks passed
@urbanophile
Copy link

This is great! Thank you @Jerakin and @Linbreux!

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.

3 participants